1ca55ac77d chacha20_poly1305: inline simd functions (Nick Johnson)
30920c4d84 chacha20_poly1305: drop mutable requirement (Nick Johnson)
415945cd2b chacha20_poly1305: avoid duplicate block work (Nick Johnson)
33dc1b95fa chacha20_poly1305: swap tuple for array (Nick Johnson)
dadd1d7224 chacha20_poly1305: remove alignment (Nick Johnson)
36d45bf360 chacha20_poly1305: remove mod operator (Nick Johnson)
Pull request description:
Inspired by JeremiahR's new benchmark on the chacha20 module, I dug in and found a few tweaks (and a larger refactor) which help a lot with performance.
The tweaks are broken down by commit. One of the relatively fruitful ones (5% increase) was refactoring the U32x4's rotation methods to use hardcoded matches instead of the mod operator. Others saw more modest performance gains.
The big change though was to *not* calculate two blocks for each keystream call (oops). I refactored the logic to handle the offset state (which is technically not required for BIP324, but as seen here, can be confusing to not have) and now we always calculate just the required amount of a keystream. This about doubles the performance.
For the curious, the SIMD performance isn't very impressive at this point, just a modest increase. But I have some hope that the experimental core library U32x4 will bring a nice bump (it is implemented with some unsafe hacking) and be an easy refactor.
ACKs for top commit:
apoelstra:
ACK 1ca55ac77db698f3816d8b7ed4051ddb5a579a29; successfully ran local tests
tcharding:
ACK 1ca55ac77d
Tree-SHA512: 8db1c6144d172775164859ffd0d97021f90d9123d06b5cccde21800aec4dcea75a5753d547933288ae233caf82e92849f894fb7019537d188baf10fbf7019684
We recently added a bunch of types in the `block` module that are
wrappers around `u32`. When we did we slapped `pub` on the inner fields.
Lets be more mindful and make the inner fields private. Note all these
types have `to_u32`, `from_u32` and `From` in both directions.
e2d03fef72 units: Manually implement serde traits (Tobin C. Harding)
Pull request description:
For types that have private inner fields we would like to manually implement `serde` traits instead of deriving them.
This hardens the crate against making future mistakes if we want to change the inner fields.
Do so for:
- `Weight`
- `BlockTime`
- `NumberOfBlocks`
- `NumberOf512seconds`
ACKs for top commit:
apoelstra:
ACK e2d03fef72f4fcbdacd1cbecc0c2dddfbdeb3031; successfully ran local tests
Tree-SHA512: 6d3cc4106e44e08d59f8da6f8f3285923408ed05d1c0b2f6f2d83a22718744a38bf67381177856b5713810a5f5318d929daa2d12447c722628e0576fcd075e3d
980365097d Fix expecting string for amount types (Tobin C. Harding)
3658865a18 Fix expecting string for fee_rate (Tobin C. Harding)
d6940497fd Simplify fee_rate serde deserialize opt (Tobin C. Harding)
87d6f1718c Make serde attribute usage more terse (Tobin C. Harding)
Pull request description:
Clean up the `serde` stuff for `FeeRate` and `Amount`. This is all pretty trivial.
ACKs for top commit:
apoelstra:
ACK 980365097d44fc8ebb8e0b1327058a59ae62a439; successfully ran local tests
Tree-SHA512: 70d3b5245f69c87b03ed2259bb6eefc93d075c6246f1fae3e4cc14934f41bbcf6cb6d69b3a3986b3b382bb00f3457a2f7be91e8b805823db4f802a086f2f27e4
For types that have private inner fields we would like to manually
implement `serde` traits instead of deriving them.
This hardens the crate against making future mistakes if we want to
change the inner fields.
Do so for:
- `Weight`
- `BlockTime`
- `NumberOfBlocks`
- `NumberOf512seconds`
According to the `serde` docs:
> This is used in error messages. The message should complete the
> sentence “This Visitor expects to receive …”, for example the message
> could be “an integer between 0 and 64”. The message should not be
> capitalized and should not end with a period.
Use a lower case character to start the string.
According to the `serde` docs:
> This is used in error messages. The message should complete the
> sentence “This Visitor expects to receive …”, for example the message
> could be “an integer between 0 and 64”. The message should not be
> capitalized and should not end with a period.
However we have the `expecting` str using the converted type not the
thing the visitor expects.
Use `u64` instead of `FeeRate` since that is what is being parsed. Note
that in `amount` we got it _almost_ correct, subsequent patch will fix
the case.
642c414f56 refactor: Use map combinator instead of match (yancy)
Pull request description:
Use map combinator instead of match
ACKs for top commit:
apoelstra:
ACK 642c414f56b03222d25fb9f11260029c5ffb6a9a; successfully ran local tests
tcharding:
ACK 642c414f56
Tree-SHA512: b107315bb3ac944b71b2aa0de60005e5c9cfd2affda725e795dae9e4c190a5ccabf657211199bd2e884aed63e5335ffe79281e9060025103e14d5c7604f229d1
fe577cd04e Implement Div<NonZeroU64|I64> for Amount, SignedAmount, FeeRate, and Weight (frankomosh)
Pull request description:
The pr implements `Div<NonZeroU64>` and `Div<NonZeroI64>` for the following types in `units` crate: `Amount`, `SignedAmount`, `FeeRate`, `Weight`
For handling owned/borrowed variants, each impl is wrapped in the existing `impl_op_for_references!` macro, which generates: `T / NonZero*` , `&T / NonZero*`, `T / &NonZero*`, `&T / &NonZero*`
close: #4442
ACKs for top commit:
Kixunil:
ACK fe577cd04e
apoelstra:
ACK fe577cd04e64488371aa62b872e2b88050d4948f; successfully ran local tests
tcharding:
ACK fe577cd04e
Tree-SHA512: b9b4a9f46d2fcf559d0a7f62ec397b6c2b174dd8ca9d80b37c0c393894ab4bc32019d64e2afcad1f780442b16aa3d15240ce607fc14b5e17b112243f7556b5b4
We (I) have recently done to PRs patching the way we handle converting
scripts to and from hex.
In doing these I made a mess of the deprecation because after both PRs
were in I had managed to change the return type and the behaviour of the
deprecated function.
On top of that the docs were either outright wrong or not that clear.
Props to Kix for doing post merge review and finding my mistakes.
c3102f04de Fix link to BIP-113 (Tobin C. Harding)
Pull request description:
Current rustdocs link is not correct. Just remove the brackets because we have a fully labelled link right below.
ACKs for top commit:
apoelstra:
ACK c3102f04dea27388e830e619a43e8b1130bd2d08; successfully ran local tests
Tree-SHA512: 71e41b3bd929fd8ed4e6aa790d73ba33132856dc1b3b7d36afde73f94c1953898814f851caf7004ba1a456d3af107513733a4df810c1b1ce74a3e332b156d97b
d003d48592 Add `Builder::with_capacity()` (Daniel Roberts)
e492f94289 Update `ScriptBuf::with_capacity` docs (Daniel Roberts)
Pull request description:
Enable the creation of `bitcoin::script::Builder` with a preallocated capacity.
This is pretty minor, but it provides a small speedup if used correctly. I've observed a 0.4% speedup and a 0.7% speedup in two code bases that create lots of outputs (on the order of tens to hundreds of thousands). It provided a much larger speedup (5% or so) in the latter code base before some other optimizations dwarfed it.
I have a branch that also uses it for `ScriptBufExt::new_*()` but while it provides a small performance improvement for all script types except one, `ScriptBufExt::new_p2tr()` actually causes a small performance regression. Since the only use case I have for creating lots and lots of scripts is in taproot with CHECKTEMPLATEVERIFY, I'm holding back that change if/until I figure out what's going on exactly (and hopefully resolve it).
ACKs for top commit:
tcharding:
ACK d003d48592
apoelstra:
ACK d003d4859251abb01929ccab6009a02e0c96b0cd; successfully ran local tests
Tree-SHA512: 51137574ca3d9dc5c319df124f470abd0f82413c093a5636af0439eb0fc2ad01dcf83df366a02279fc7d28feed24aa8c7db33b3c474d8bde7a9b6636343f8e9a
Adds an implementation of div by NonZeroU64 for Amount, FeeRate, and Weight
types. Also adds a div by NonZeroI64 for SignedAmount. The operations
helps to prevent div-by-zero errors at compile time, rather than runtime.
It follows same pattern as existing div operations
but leverages safety guarantees offered by non-zero types
3fbd6fb6b3 test: Add constructor test (yancy)
Pull request description:
The constructor currently has no test coverage.
ACKs for top commit:
apoelstra:
ACK 3fbd6fb6b3c6dc1f1d5c14b3f3486140a781d0f0; successfully ran local tests
tcharding:
ACK 3fbd6fb6b3
Tree-SHA512: 9819bd058b84a7b633279d12da48c9af7af765fd8dca5d297787872badc431769c026e57b03bf6d907c89a7b7a5c116cda1be98cb6261dd2a6c331276e627cc3
a66ff8f8b1 Introduce `map` function for `NumOpResult` (Shing Him Ng)
Pull request description:
Closes#4476
ACKs for top commit:
Kixunil:
ACK a66ff8f8b1
apoelstra:
ACK a66ff8f8b143ebebb80ddc6b052cbc55e8dbb070; successfully ran local tests
Tree-SHA512: 63c79666685895033b9df0c46004fa4b042d038cc61e5ef443f56690be268ac6dd1ba461ab4f7d97c684e68623dfa53cdd37091f40ff7e6a5d3e53920c3fd40c
873880b192 test: push int minimality (ChrisCho-H)
Pull request description:
Integrate the minimality test of `push_int` into that of `push_slice`. This increases test coverage.
ACKs for top commit:
apoelstra:
utACK 873880b192
tcharding:
ACK 873880b192
Tree-SHA512: 8bbd0b7ec4c69faaadb9ab4bae7429bbebd66d1d718b80f19b323a1059a983ea1b41f743a920b6fdacce213e66708ed1028227246021457601bce968b8bf3f22
03b8e08752 Fix argument check in labeler script (Fallengirl)
Pull request description:
### Description
Removed unnecessary quotes around the `!=` operator in the Bash test expression, so the `--force` flag is now evaluated correctly instead of always passing.
ACKs for top commit:
apoelstra:
ACK 03b8e08752e77742374b367a3947d987b81797fa; successfully ran local tests
Tree-SHA512: 479763d384a2ee06dc2a2fdd4cf3d35d1731ffccdee8888104bf0fd4bc778ee493d2e4e56f33c544c66e987a637c994036b237661e21ec02d4d86e0586ba8cb4
b038520c4d Change the return type of effective_value (yancy)
Pull request description:
Prefer the more informative return type NumOpResult over Option. Also a returns section was added which describes the different possible returns.
The api of NumOpResult could probably be extended to cleanup the match statement. Also consider api addition for unchecked calculations natively.
ACKs for top commit:
tcharding:
ACK b038520c4d
apoelstra:
ACK b038520c4dc8c2f92cb40a9fd272608ae2e9b799; successfully ran local tests
Tree-SHA512: 2e66a3ca83514f0ad011660178f8e5ea9d9b1de03dc030e7c57f558e08a42261724251364bb7a746b6970b4288a45539a33d3b36b114c855b6246a56fee3c61c
fd90c8782a Mark method as constant (yancy)
Pull request description:
Allow external const calls to access this method
Followup from https://github.com/rust-bitcoin/rust-bitcoin/pull/4428
ACKs for top commit:
tcharding:
ACK fd90c8782a
apoelstra:
ACK fd90c8782ab602861a715c72ca1481ed963e5c39; successfully ran local tests
Tree-SHA512: 2d04da9bddd58040972942e70096b2714405740236889f0909c00cb6993e38f3ae51ff05bbda13792a31243d3598f5976649590eecb0572c4f00c166f717399d
f732b1d3cc units: Use functional style (Tobin C. Harding)
Pull request description:
This is Rust not C, use functional style.
Close: #4464
ACKs for top commit:
apoelstra:
ACK f732b1d3cc2b3f9d012b3eeecc70d007666d4f1d; successfully ran local tests
Tree-SHA512: 65f523d4632c78bb166b9e4fb49e715e92da28fc2d07d54a3df8e0db7097277958dfbde0989cf1e443e6f56635001298b47fef3eb5e76b8350041b1ee7be46a6
3b8164139f primitives: Add docs section for script hex API (Tobin C. Harding)
6b90e42e78 Finalize the script hex APIs (Tobin C. Harding)
Pull request description:
In #4316 we made some 'improvements' to what script functions and trait implementations do and do not include the length prefix. Iterate again on it as described here: https://github.com/rust-bitcoin/rust-bitcoin/pull/4316#issuecomment-2847710436
- Patch 1 does the changes
- Patch 2 adds some more docs, requires a grammarian to check my Aussie lingua
ACKs for top commit:
apoelstra:
ACK 3b8164139f6ecebc97b66a299b4a87c2288d8a76; successfully ran local tests
Tree-SHA512: 481db88ae1b6f5751e81e4cd126f545cfc34bef6dcfcf857f1c7464aeb41f5de95fc4582c015abde04372fe025efa13cdf2906e75517f62cff3ddec05c4d9711
When checking a locktime against block height we add 1 because when the
next block is being validated that is what the height will be and
`is_satisfied_by` is defined to return true if a transaction with this
locktime can be included in the next block.
As we have in `relative`, and for the same reasons, add an API to the
absolute `Height` and `MedianTimePast`: `is_satisfied_by`. Also add
`is_satisfied_by_{height, time}` variants to `absolute::LockTime`.
Call through to the new functions in `units`.
Define 'is satisfied by' - this is a classic off-by-one problem, if a
relative lock is satisfied does that mean it can go in this block or the
next? Its most useful if it means 'it can go in the next' and this is
how relative height and MTP are used in Core.
Ramifications:
- When checking a time based lock we check against the chain tip MTP,
then when Core verifies a block with the output in it it uses the
previous block (and this is still the chain tip).
- When checking a height base lock we check against chain tip height + 1
because Core checks against height of the block being verified.
Additionally we currently have a false negative in the satisfaction
functions when the `crate` type (height or MTP) is to big to fit in a
u16 - in this case we should return true not false because a value too
big definitely is > the lock value.
One final API paper cut - currently if the caller puts the args in the
wrong order they get a false negative instead of an error.
Fix all this by making the satisfaction functions return errors, update
the docs to explicitly define 'satisfaction'.
For now remove the examples in rustdocs, we can circle back to these
once the dust settles.
API test of Errors:
Some of the errors are being 'API tested' tested in `primitives` but
they should be being done in `units/tests/api.rs` - put all the new
errors in the correct places.
We recently improved the relative locktime function `is_satisfied_by` by
adding mined at and chain tip. We can now do the same for the
height/time satisfaction functions.
Note I believe these functions should still be provided because a user
may for some reason have either blocktime data or height data and not
have the other.
Requires some work to the errors, elect to just remove the original
field that held the function argument.
For now remove the examples in rustdocs, we can circle back to these
once the dust settles.
06096d2a66 Automated update to Github CI to rustc nightly-2025-05-09 (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 06096d2a66
Tree-SHA512: 987f6ae0f4a54cf94c58a1e90d7e99b7687a1d0c7fecab3cde9bc0acd75b25a5336c2eb6445200fe8288e3e0af038123a717541b823a215301db7412bb555bf4
Luis Schwab
merge-script
Nick Johnson
Tobin C. Harding
yancy
frankomosh
Shing Him Ng
ChrisCho-H
Fmt Bot
Daniel Roberts
Fallengirl