- adds an item that all error messages
should be lower case, except for proper nouns and variable names.
- adds a section on expect messages,
following discussion in #3019 and #3053.
2b56f763d0 hashes: Remove to/from/as_raw_hash (Tobin C. Harding)
Pull request description:
In an effort to shrink the API of `hashes` remove the `from_raw_hash`, `to_raw_hash`, and `as_raw_hash` inherent functions from types created with the `hash_newtype` macro.
There are a few reasons why this is favourable:
- It allows stable crates to use the macro and not expose unstable `hashes` types in their API.
- It makes types created with the macro less "general" in the sense that its more obscure to just hash any data into them. This allows us to write cleaner APIs in `rust-bitcoin`.
ACKs for top commit:
Kixunil:
ACK 2b56f763d0
apoelstra:
ACK 2b56f763d0
Tree-SHA512: 3d73aa8250dd775994623c9201dd819256acf2ec82526b3537da74c9e19c2ac5e8bba358a2171f7b02342804cb6b4d5ac4dca88d912b3d46d14e3bc35dd5cb91
164b72e07b Update Key documentation (Shing Him Ng)
Pull request description:
The documentation for `Key` was a bit confusing since `<key> := <keylen> <keytype> <keydata>` was right above the `key` field, when in reality the `key` field represents the key data. Moving the documentation that describes a `Key` as a whole to the struct itself and specifying that `key` represents key data will hopefully clear things up a bit
ACKs for top commit:
Kixunil:
ACK 164b72e07b
tcharding:
ACK 164b72e07b
Tree-SHA512: 33c2b24d3006a0ed85a5d96351e0a01c1365c8ea01472233b024de54941319cdbefa0126f8b9538385e8f33ba7e2e3895f0dc5b6a36a1c501c8b97ebbede6502
8dcecfc144 Remove midstate from the GeneralHash and HashEngine traits (Tobin C. Harding)
Pull request description:
Midstates are not generic objects; they don't have universal cryptographic properties and if you are using them you should be using a specific midstate type. Therefore it shouldn't be part of `GeneralHash` or `HashEngine`. Furthermore, in practice it seems like `sha2` midstates are the only ones that anybody uses, at least in bitcoin.
Remove the midstate stuff from the `GeneralHash` and `HashEngine` traits. Keep the `midstate` functionality as inherent functions if it is used internally. Keep the functionality on `sha256` as inherent public functions.
Done as a first step towards #2918.
ACKs for top commit:
apoelstra:
ACK 8dcecfc144
Kixunil:
ACK 8dcecfc144
Tree-SHA512: 495fe6a4d852ecf51c21acc1ae7215f1914f6d12377eb7e6da678b8ffb2c3c8b06976aa5290010a88d5a9cbc56752fcbdf740cb27b22ada8079a69dcf2e32dc0
bcf6d2839e Introduce scriptPubkey extension traits (Tobin C. Harding)
ee333defa4 Remove path from ScriptBuf (Tobin C. Harding)
Pull request description:
Done in preparation for moving the script types to `primitives`.
Add three public traits, one each for the three `script` types: `Builder`, `ScriptBuf`, and `Script`.
ACKs for top commit:
Kixunil:
ACK bcf6d2839e
apoelstra:
ACK bcf6d2839e
Tree-SHA512: 0ad11e474ddf1183d0119e36454cb4fd18d49a68655d274df800c6ef20afa7f8d0fdecd415c02595ea67a011e3a842b7ccc23c2d58f92ed9acbdc7f277fbd217
e2b0fd33be Specify required_height in variable name when comparing to other height (Shing Him Ng)
Pull request description:
Update the variable names for `h`/`height` to `required_height` in instances where it was being used to create another height or used comparatively i.e. `required_height + 1` or `required_height < height`
Resolves#2553
ACKs for top commit:
Kixunil:
ACK e2b0fd33be
tcharding:
ACK e2b0fd33be
Tree-SHA512: 05fa8df721148d9bbc6eaa2b14c2583d3415c3ab0d53b8fdf39d7b9e9a28463265e43abfc430dfc7f6d0b53cd17767f5cad83c2de73b8a153aeb6befeeac84bc
In an effort to shrink the API of `hashes` remove the `from_raw_hash`,
`to_raw_hash`, and `as_raw_hash` inherent functions from types created
with the `hash_newtype` macro.
There are a few reasons why this is favourable:
- It allows stable crates to use the macro and not expose unstable
`hashes` types in their API.
- It makes types created with the macro less "general" in the sense that
its more obscure to just hash any data into them. This allows us to
write cleaner APIs in `rust-bitcoin`.
Done in preparation for moving the script types to `primitives`.
The script types have a bunch of functionality to support scriptPubkeys,
and scriptPubkeys are an address thing.
Create a module under `address` and in it create a bunch of extension
traits to hold all scriptPubkey functionality.
Includes adding an ugly-as-hell macro to create the traits.
Midstates are not generic objects; they don't have universal
cryptographic properties and if you are using them you should be using a
specific midstate type. Therefore it shouldn't be part of `GeneralHash` or
`HashEngine`. Furthermore, in practice it seems like `sha2` midstates are
the only ones that anybody uses, at least in bitcoin.
Remove the midstate stuff from the `GeneralHash` and `HashEngine`
traits. Keep the `midstate` functionality as inherent functions if it is
used internally. Keep the functionality on `sha256` as inherent public
functions.
360d1fb1bb hashes: Use $crate in macro (Tobin C. Harding)
Pull request description:
Depending on types being in scope when calling macros is bad practice but we have mistakenly done so in `internal_macros` when using the `FromSliceError`.
Use `$crate::FromSliceError` in the macro and remove import statements.
ACKs for top commit:
Kixunil:
ACK 360d1fb1bb
apoelstra:
ACK 360d1fb1bb
Tree-SHA512: e93264ed7313e6f128cc9a441f68100fe423263aa9677c194d768cf1b04058bfc840027540e1c9d82ff5e925ddc165f34b2c6926987f610caa1686ea27a955eb
ac4db6369d witness: Add Witness::witness_script inspector (Steven Roose)
6cc6c8621a witness: Add Witness::taproot_annex (Steven Roose)
b0848022eb witness: Add Witness::taproot_control_block (Steven Roose)
ef336e1387 witness: Improve Witness::tapscript (Steven Roose)
e48a2e4225 script: Add Script::redeem_script inspector (Steven Roose)
Pull request description:
Bundled these because they are very similar. Got a bunch of larger changes coming up based on these. I've been using these for a while for TXHASH work.
ACKs for top commit:
apoelstra:
ACK ac4db6369d but will need to wait for next release. I think we should merge these as-is although they will be much clearer after we do script tagging.
tcharding:
ACK ac4db6369d
Tree-SHA512: e1590d1bdc8b91aeba137453f0cdaa7e1ae6df3c8e9e1e0f087ed9be1a6beaf2286818379247d26c5dd27d07c12c10433db1c9b9a71667ab4d8d37c7deff1373
e2c3694184 rustfmt: Use show_parse_errors (Tobin C. Harding)
Pull request description:
There are no behaviour changes with this patch applied.
In https://github.com/rust-lang/rustfmt/pull/5961 the `hide_parse_errors` was changed to `show_parse_errors` with default `true`.
Since we run the formatter with nightly we are getting the following warning
Warning: the `hide_parse_errors` option is deprecated. Use
`show_parse_errors` instead
Rename the variable and keep the default value `true`.
ACKs for top commit:
Kixunil:
ACK e2c3694184
storopoli:
ACK e2c3694184
apoelstra:
ACK e2c3694184
Tree-SHA512: f75f3abf194d65e0ef2a1bef2544bc94ed1c4911b94cbc6bc5700ecb8e6ec87c5564b9711a7dde8ca1a83400e1bf3154d25574a485e9893049be0a46f2c95e33
Depending on types being in scope when calling macros is bad practice
but we have mistakenly done so in `internal_macros` when using the
`FromSliceError`.
Use `$crate::FromSliceError` in the macro and remove import statements.
dc96475f58 Add/fix alloc features (Tobin C. Harding)
Pull request description:
Eventually we would like all our crates other than `bitcoin` to be able to be used without an allocator. Currently, and during crate smashing, this is not that useful because so much of the code comes from `bitcoin` and relies on the availability of an allocator.
As an initial step, add the `alloc` feature to `addresses` , `base58`, and `primitives`.
In order to to keep `--no-default-features` builds working make the crates empty if the `alloc` feature is not enabled. This is a suboptimal solution because the error messages users will get when they forget to enable `alloc` will be confusing (eg something like primitives does not contain Transaction). However our CI script (`run_task.sh`) expects `--no-default-features` to build cleanly (as do I).
ACKs for top commit:
apoelstra:
ACK dc96475f58
Kixunil:
ACK dc96475f58
Tree-SHA512: 28006ad638e72d3c712becaf94f6aaddc559fb2f7e7ad0d0810348fe979fb61e32f53e5b20894e472c5ac9e2b7f80cba6a3f0e12b766b817f412a927957ae3a2
40bee24de7 Run the formatter (Tobin C. Harding)
Pull request description:
I don't want to wait for the bot, just run the formatter now.
No manual changes.
ACKs for top commit:
apoelstra:
ACK 40bee24de7 fine by me, but will give Kix 2 days to express displeasure, at which point the bot will pretty-much be ready to run
Kixunil:
ACK 40bee24de7
Tree-SHA512: a9e54305062900dc18f73c61deef0af74702c17f7040614f5f8697a47dd084d07a9c321878c5e29910e2a8d129bac23b95ee1023176beddf2e915544b13e7b0c
9933fa0dd5 contributing: fix instructions for the semver CI (Jose Storopoli)
213566f34b ci: semver-checks should run on rust stable (Jose Storopoli)
0fa2b0b16a ci: add all-features semver-checks (Jose Storopoli)
Pull request description:
Crates missing:
- `bitcoin_hashes`
- `bitcoin-units`
- `bitcoin-io`
Also adds:
1. corrects a small error in the CONTRIBUTING.md; and
2. runs on `stable` since `cargo-semver-checks` do not support `nightly` (rustdocs JSON format breaks almost daily)
Closes#2999.
ACKs for top commit:
Kixunil:
ACK 9933fa0dd5
tcharding:
ACK 9933fa0dd5
Tree-SHA512: c5251cec7c7189e7ef16a657ef35723b2c3e8fa7262c452fc83828dc16215107259d445f911916ec0716ea6e20ce59d0910b9a4fa004f150322fd4fcd00d15c5
d05723c401 Optimize base58 on small inputs (Martin Habovstiak)
Pull request description:
Most base58 strings in Bitcoin are somewhat short. There was previously an "optimization" putting part of the input on stack which was removed in #2759 because it actually made the code slower. This appears to be mostly because of branches caused by using `iter::Chain`.
Manually splitting the iterations into two helped bring the performance close to what #2759 achieved but that still wasn't worth it. But given that we know the input length in many cases (it's just a slice) we can determine whether it'll fit a buffer upfront and then just call different functions which don't have the branches in loops. To avoid having two functions this uses generics instead. Further, we increase the buffer length to 128 and use `ArrayVec` from `internals` which internally avoids initializing the buffer thanks to `MaybeUninit`
In total this increases performance by around 4% on my machine.
ACKs for top commit:
tcharding:
ACK d05723c401
apoelstra:
ACK d05723c401
Tree-SHA512: a12fa15ef2b58282a69545685dbbe027d4e73a285651a9fe05de5a039a11c5da2fc1000eb369801e77584cd403bb0e972f5f01bcab0d1e62624f054992b8b83d
Most base58 strings in Bitcoin are somewhat short. There was previously
an "optimization" putting part of the input on stack which was removed
in #2759 because it actually made the code slower. This appears to be
mostly because of branches caused by using `iter::Chain`.
Manually splitting the iterations into two helped bring the performance
close to what #2759 achieved but that still wasn't worth it. But given
that we know the input length in many cases (it's just a slice) we can
determine whether it'll fit a buffer upfront and then just call
different functions which don't have the branches in loops. To avoid
having two functions this uses generics instead. Further, we increase
the buffer length to 128 and use `ArrayVec` from `internals` which
internally avoids initializing the buffer thanks to `MaybeUninit`
In total this increases performance by around 4% on my machine.
1324ae88ee Automated update to Github CI to rustc nightly-2024-07-10 (Update Nightly Rustc Bot)
Pull request description:
Automated update to Github CI workflow `rust.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
tcharding:
ACK 1324ae88ee
Tree-SHA512: 8689a061a578f6f435d4e3f885d6b952b8d7d0c618928ba5b7f7a92ad2311f2799251f72ab3b2170f97ae390605f2b6b29b1249c70b7e53933006be23b3ae6cf
There are no behaviour changes with this patch applied.
In https://github.com/rust-lang/rustfmt/pull/5961 the
`hide_parse_errors` was changed to `show_parse_errors` with default
`true`.
Since we run the formatter with nightly we are getting the following
warning
Warning: the `hide_parse_errors` option is deprecated. Use
`show_parse_errors` instead
Rename the variable and keep the default value `true`.
Eventually we would like all our crates other than `bitcoin` to be able
to be used without an allocator. Currently, and during crate smashing,
this is not that useful because so much of the code comes from `bitcoin`
and relies on the availability of an allocator.
As an initial step, add the `alloc` feature to `addresses` , `base58`,
and `primitives`.
In order to to keep `--no-default-features` builds working make the
crates empty if the `alloc` feature is not enabled. This is a suboptimal
solution because the error messages users will get when they forget to
enable `alloc` will be confusing (eg something like primitives does not
contain Transaction). However our CI script (`run_task.sh`) expects
`--no-default-features` to build cleanly (as do I).
1cce1b5aa6 Remove private prelude module from units crate (Jamil Lambert, PhD)
Pull request description:
The private prelude module has been removed from the units crate and instead imports are stated in full when needed. As discussed in #2926.
ACKs for top commit:
Kixunil:
ACK 1cce1b5aa6
apoelstra:
ACK 1cce1b5aa6
Tree-SHA512: 58b93ff66f74399938bc1a7f59fe8d2a21d0437c7e90e0c190d3d6a8de30f9c9268c8e4288d1db287b4d190624968937b1ad6c6e54d29025370e47e71be925c1
c2bdc68f5e internals: Remove derive from test-serde (Tobin C. Harding)
Pull request description:
During review of #2889 it was noted that we don't need to enable the `derive` feature of `serde` in the `test-serde` feature.
Do not enable `derive` in the `test-serde` feature.
ACKs for top commit:
apoelstra:
ACK c2bdc68f5e nice
Kixunil:
ACK c2bdc68f5e
Tree-SHA512: 64def07c2ae93fd98fae10a51b978c9777b43b4e153a4f42c6d7dfa6ba7d65b98a313f08337bb3ace3078a319cc450e23717c481ba631aca5ed84b098dcf36e4
a738754f67 Add TxIdentifier trait (Tobin C. Harding)
Pull request description:
Add a new trait `TxIdentifier` that abstracts over the `Txid` and `Wtxid` types. We make `AsRef` a super trait so that the new trait needs no methods.
Seal the trait so consumers of the library cannot implement it.
Use the new trait in:
- the `bip152` module to tighten up the `with_siphash_keys` function
- as a trait bound on the `Leaf` associated type in the `MerkleNode` trait
ACKs for top commit:
apoelstra:
ACK a738754f67
Kixunil:
ACK a738754f67
Tree-SHA512: a7bda26a4a5107f96b24ea3c163286a7ab21a817bdec3434b3ab27d78e99c0548a7362a2271d362b89038c80d9251767c0d62e1df702ef57d9edaf141ab55cd6
51010777bf hashes: Strongly type the hash160::HashEngine (Tobin C. Harding)
d5dd54a489 hashes: Strongly type the sha256d::HashEngine (Tobin C. Harding)
a7422a779c hashes: Add const hash engine constructors (Tobin C. Harding)
Pull request description:
Currently we are using a type alias for a few of the hash engines.
Type alias' allow for potential mixing of types, a struct can better serve our users with not much additional complexity or maintenance burden.
- As preparation add const constructors where possible to hash engines (excl. `HmacEngine`).
- Add a `sha256d::HashEngine` struct
- Add a `hash160::HashEngine` struct
If this goes in we can improve the `sha256t::HashEngine` in a similar manner.
ACKs for top commit:
Kixunil:
ACK 51010777bf
apoelstra:
ACK 51010777bf but will hold off on merging until tcharding indicates whether he wants to address the midstate thing
Tree-SHA512: 810db3a8cd66e4d135b04a65d5b4c926cf2e5e8ac531619bbd884db69df17b29b64daeb6fb31b8b1fb32bffbf81cf84e55cd46945c743451c73f1b7f63489f63
9a586987d1 Move opcodes to primitives (Tobin C. Harding)
Pull request description:
Move the `opcodes` module to the new `primitives` crate. This is pretty straight forward, some things to note:
- Are we ok with the public wildcard re-export from `blockdata`? I think so because the whole `blockdata` module should, IMO, be deleted after everything in it is moved to `primitives`.
- `decode_pushnum` becomes public.
ACKs for top commit:
Kixunil:
ACK 9a586987d1
apoelstra:
ACK 9a586987d1
Tree-SHA512: ee9fa0ae4265f54ff7784dc873abc12572852c32ff24456e34cd6a8a004f9e1f932e01c80d3448107fca76507db4bdaa3dfff6b5a80de0707d59a033e582fb9e
afd19ebd61 Use super for imports in script module (Tobin C. Harding)
Pull request description:
In the `script` module we currently import `script` types using the fully qualified path, as recently discussed code is easier to maintain if we use `super` when `super != crate`.
Internal change only, no external changes.
ACKs for top commit:
Kixunil:
ACK afd19ebd61
apoelstra:
ACK afd19ebd61
Tree-SHA512: 5d8546de3d8d9014054fb53020ec0d742eb9698d6d70cd0b2e722b5e43792f4d1ad480d1c61d67b10ddda6a5ef42839118f0de0332251bd64cf31f0a566fd40b
f8c91b067b ci: move semver-checks to its own job (Jose Storopoli)
Pull request description:
This is necessary because it needs to run only on pull_request, and not on push events.
Closes#2968.
ACKs for top commit:
apoelstra:
ACK f8c91b067b
tcharding:
ACK f8c91b067b
Kixunil:
ACK f8c91b067b
Tree-SHA512: 31b8817c2cc6aaf4902666140fba51ae1b2f54cc8843eacbe2782dbfca526023c536064351e08b58e5e1d999eb08937a39f619a36e3d9107fd688dea313638d2
6dd5af9678 Add missing links (Jamil Lambert, PhD)
47e367f011 Standardize error headings (Jamil Lambert, PhD)
75f317a689 Fix rustdoc grammar (Jamil Lambert, PhD)
573f8ce724 Add backticks to rustdoc links (Jamil Lambert, PhD)
Pull request description:
The rustdocs in the `units` crate has been reviewed and improved.
Some of the links were missing backticks, these have been added.
Some grammatical changes have been made to improve the documentation.
The use of links was inconsistent and has been changed to have links everywhere that seemed appropriate.
A couple of error headings were added and a description as to why a capital M is not accepted in the denomination for MegaSatoshi or MegaBitcoin.
ACKs for top commit:
apoelstra:
ACK 6dd5af9678
tcharding:
ACK 6dd5af9678
Tree-SHA512: f5481a7c3aa99d7882cda9ccda9df9e27f092ff91ef584f07dc887f38bfe12e5ea801cfa7f42bb4022301860489ee6be55557752a8cbe70932f258ea753495dc
Move the `opcodes` module to the new `primitives` crate. This is pretty
straight forward, some things to note:
- Are we ok with the public wildcard re-export from `blockdata`? I think
so because the whole `blockdata` module should, IMO, be deleted after
everything in it is moved to `primitives`.
- `decode_pushnum` becomes public.
Includes addition of a `patch` section for `primitives` in the
`bitcoin/embedded` crate.
In the `script` module we currently import `script` types using the
fully qualified path, as recently discussed code is easier to maintain
if we use `super` when `super != crate`.
Internal change only, no external changes.
f5dcbc1723 Automated update to Github CI to rustc nightly-2024-07-07 (Update Nightly Rustc Bot)
Pull request description:
Automated update to Github CI workflow `rust.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
tcharding:
ACK f5dcbc1723
Tree-SHA512: 094407c926fa79fe22d26253296076c80b58240e62de5921f5ed4aee67177d3fb3711f0d40c2bd422b834a88df62ab4e9442165c2826ad1f787ad2d3fb23b2a4
Currently we are using a type alias for the `hash160::HashEngine`.
Type alias' allow for potential mixing of types, a `hash160::HashEngine`
struct can better serve our users with not much additional complexity or
maintenance burden.
As we did for the `sha256d::HashEngine`, add a new wrapper type
`hash160::HashEngine` that replaces the current type alias.
Currently we are using a type alias for the `sha256d::HashEngine`.
Type alias' allow for potential mixing of types, a `sha256d::HashEngine`
struct can better serve our users with not much additional complexity or
maintenance burden.
Add a new trait `TxIdentifier` that abstracts over the `Txid` and
`Wtxid` types. We make `AsRef` a super trait so that the new trait needs
no methods.
Seal the trait so consumers of the library cannot implement it.
Use the new trait in:
- the `bip152` module to tighten up the `with_siphash_keys` function
- as a trait bound on the `Leaf` associated type in the `MerkleNode` trait
Jose Storopoli
merge-script
Fmt Bot
Tobin C. Harding
Ryan Breen
Shing Him Ng
Martin Habovstiak
Jamil Lambert, PhD