Mark `checked_` functions const. Replace `map()` and `?` operators,
which are not allowed in const context, with match statements.
Use descriptive variable names in ceiling division to make
it easier to follow.
7876a0d5ee Automated update to Github CI to rustc nightly-2024-11-17 (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 7876a0d5ee
Tree-SHA512: 80f238cd5a8b5ddf7c71bc8d80cf6cbf930b3db615fbca2a52d59c8f57b38b0ef106d5480a6c18c37821fe58f85dfa8c950a238933376d82d831a08cc78dc46e
68aac98753 Mark functions `const` (Jamil Lambert, PhD)
Pull request description:
Following on from #3608 and #1174 more functions have been marked as const.
Mark `checked_` functions in `units/src/fee_rate.rs` as const.
Replace `map()` and `?` operators, which are not allowed in const context, with match statements.
ACKs for top commit:
apoelstra:
ACK 68aac98753dba77bccc6572c65d441108dc53a17; successfully ran local tests; nice
tcharding:
ACK 68aac98753
Tree-SHA512: 2165f0fdb2bbbd68223646c7f788ec3b637cf2bec06a9faee8b252900fac0517985047d5e88ec074b6d9121fcb3a3774a62f326e53e1ceaa041d65a4e2e35fb6
ad82ed7179 Fix spelling typo (yancy)
8fe5ffde4c Rename tests that have _test suffix (yancy)
Pull request description:
Convention is to not include test as a suffix
Used `git grep -n _test` and renamed all the tests that made sense to rename. There are a number of fn `do_test(data: &[u8]) {` that I felt it would be better not to touch. Everything else that was a testname I renamed if it had a _test suffix.
ACKs for top commit:
apoelstra:
ACK ad82ed71796c79399cd8a81814b1af61c4ca3582; successfully ran local tests
tcharding:
ACK ad82ed7179
Tree-SHA512: 9c4ee71974e39814a8a63d269e3dcf6e761312dd0903ac1e6268dc421b9ef89a63f27cade3d5a51436660bb01457ac1a2f23a628a4d11622cd4a33fa6c483934
ec06028f63 hashes: Make hex dependency optional (Tobin C. Harding)
9dce0b4b8c Remove hex string trait bounds from GeneralHash (Tobin C. Harding)
766f498b33 Pull serde stuff out of impl_bytelike_traits macro (Tobin C. Harding)
Pull request description:
This is done in 3 parts:
1. Pull the `serde` stuff out of `impl_bytelike_traits` to fix the bug described here: https://github.com/rust-bitcoin/rust-bitcoin/issues/2654#issuecomment-2470716693
2. Prepare the `hashes` trait by removing string/hex trait bounds from `GeneralHash` and also pull the hex/string stuff out of `impl_bytelike_traits`
3. Make hex optional, including adding custom debug logic when `hex` feature is not enabled
Patch 3 is tested in `hashes/embedded`, by the new `debug` unit test, and there is a `Midstate` unit test as well that covers the `Debug` impl.
Close: #2654 - BOOM!
ACKs for top commit:
apoelstra:
ACK ec06028f63ba591a14c3a15cdfd410bb5ff1c09b; successfully ran local tests; nice!
Tree-SHA512: 85eb10d36a4581af6cd700f7ff876585bcc114c60e9864906e65659f3b3ee550fe6d9f40ca4230d870a9e23f0720723e11443ec329f16e40259a259b9be57466
53d3ab18b9 Bump version of bitcoin-primitives to 0.101.0 (Tobin C. Harding)
175171d339 Make primitives re-exports uniform (Tobin C. Harding)
3e9267071f primitives: Update crate level docs re allocator (Tobin C. Harding)
Pull request description:
Its 00:02 on the 15th UTC now, I'm hoping this merges within 24 hours - then the date is correct.
ACKs for top commit:
apoelstra:
ACK 53d3ab18b9d3385c8c72fe422a36235c825f44c3; successfully ran local tests
Tree-SHA512: c2a806867e1d50ba7512d4f1c91cd758688f587f985177a8dc3c12f495e81aa71bf1ec527611ddda0b09f271e9d652e58750ef2fa4756f447bd9be660985baab
4223655829 Add test case (yancy)
Pull request description:
Since there are test cases for other functions here, add test case as follow up for https://github.com/rust-bitcoin/rust-bitcoin/pull/3604. I forgot to request this during review.
ACKs for top commit:
apoelstra:
ACK 422365582964cf405529de3a1cf63d766b0d0e2e; successfully ran local tests
tcharding:
ACK 4223655829
Tree-SHA512: 8629cfff3e3eb800aa4d4625627e0a777c389566f4125bc63e67fe4c5a597ca63b9fb92da42f6c9dab12e6c63839eef35a13957f98606a37ed687eb96d896c2e
In preparation for releasing `primitives v0.101.0` bump the version
number, add a changelog entry, update the lock files, and depend on the
new version in all crates that depend on `primitives`.
7819e50055 Move Block to primitives (Tobin C. Harding)
55200924e4 Move Sealed to bottom of file (Tobin C. Harding)
d5b148d400 Make block_size free standing and rename (Tobin C. Harding)
5016a73207 Manually implement consensus traits for Block (Tobin C. Harding)
Pull request description:
Re-design the `Block` API and move the type to `primitives`.
ACKs for top commit:
apoelstra:
ACK 7819e50055c1be1d09b7d08734955f43229e6ed5; successfully ran local tests; let's go
Tree-SHA512: 13dc27508348966392aab4a80a3009bd53dd604d98ef392b9655bb97d669e01c4399901043ca3558b9b339cec5a1521ed0abfe7d666a66c6cc84ee4a97772de1
0348801ef3 Remove Amount::fmt_value_in (Shing Him Ng)
Pull request description:
Saw this mentioned in #3556Fixes#2952
ACKs for top commit:
apoelstra:
ACK 0348801ef30d1cdfdc40cd2316671532d4c06141; successfully ran local tests
tcharding:
ACK 0348801ef3
Tree-SHA512: 29a1d230672a22d059d2ef0870e68dec0fe6d6ae9d3fed51275d9b6779d097578a38a514544df11bdf23908c2d8e3c5fd5c6de2e460aab9792770d6b9e396fc6
On the way re-design the API by doing:
- Introduce `Checked` and `Unchecked` tags
- Rename the `txdata` field to `transactions`
- Make the `Block` fields private
- Add getters for `header` and `transactions` fields
- Move the various `compute_` methods to be free standing functions
- Make the `check_` functions private
- Introduce extension traits
In preparation for adding a block extension trait move the only private
method off the `Block` type and make it free standing.
While we are at it just take a slice of transactions since we don't need
the block itself - rename appropriately.
Internal change only.
33b9d6904c Remove From and Into test impls (Shing Him Ng)
Pull request description:
Created a new `test-utils` module for these functions since it needs to be visible for tests in the `block` module that were also using the previous `From` and `Into` impls.
Fixes#3160
ACKs for top commit:
apoelstra:
ACK 33b9d6904c699a0cdd5f14f126b977ce2d4317d6; successfully ran local tests
tcharding:
ACK 33b9d6904c
Tree-SHA512: 33f2aff7f0a468f12a8d6cd3fe375563ea5ce2870ef23f483fb9d1eeec146e1f4da66ca9ea17eb05fba7693b995d3aa00a0fd9ff0d1e86a358b6b9d7f0ee9288
971167d0fd Remove test suffix from test names (yancy)
Pull request description:
Remove test suffix from test names
ACKs for top commit:
apoelstra:
ACK 971167d0fdddf1636d29330f3b17d052c5f05323; successfully ran local tests
tcharding:
ACK 971167d0fd
Tree-SHA512: ad19d6e6eb57e61d9f3aa5790b6c5c8a6d85ec6733747d05032f2f9da94076cadbf7d293a5aa71bd0d090d1381a422c516107fea3e162e0a73385c3d07cf8f3f
f72db293f7 Automated update to Github CI to rustc nightly-2024-11-13 (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 f72db293f7
Tree-SHA512: 5936042c75fedb8bc6b2472b3d7d63a75861e854e10a475b8c85836c2194b6f8fc9f45d6a959b9ad57603d6407d2b0c25694d796352aa3abb70416407953930b
The only reason we need `hex-conservative` is to parse strings and
format them as hex. For users that do not require this functionality we
can make the `hex-conservative` crate an optional dependency.
The `serde` feature requires `Display` so we enable `hex` from the
`serde` feature.
If `hex` feature is not enabled we still need to be able to debug so
provide `fmt::Debug` functionality by way of macros.
Close: #2654
be553217f1 transaction: Add plural field getters (Tobin C. Harding)
Pull request description:
The `Transaction` type was created way back when, and the field names were named using singular (`input` and `output`). In hindsite plural probably should have been used since its more idiomatic Rust but the fields have been around so long now that re-naming them is going annoy a whole bunch of people.
As a compromise add getters that use plural, immutable and mutable versions, for both the `input` and `output` fields.
Close: #822
ACKs for top commit:
apoelstra:
ACK be553217f19069dd9ca28c3ba5953059df9caa1b; successfully ran local tests; though will let this sit a couple days before merging
sanket1729:
ACK be553217f1
Tree-SHA512: 4df9db179a69c9eea2940008c9eca9f9b9b9d9f27e2a174db39bfb3d5c32a9d06b26e5349e5baf5af78600ae128d1b534559baf7335aec6df1238de96a508764
4c94695942 Mark funtions const (yancy)
Pull request description:
May as well mark these const as well. Follow up to https://github.com/rust-bitcoin/rust-bitcoin/pull/3605
ACKs for top commit:
apoelstra:
ACK 4c94695942de41e5eca4ac120db2e069c0bb037d; successfully ran local tests; yeah, I think this are all worth consting
tcharding:
ACK 4c94695942
sanket1729:
ACK 4c94695942
Tree-SHA512: bf41e416cb8a2eb90e26d7bd640df5c2a5a868e3b721bde7cc8a3b97c687bab4d1cb4ab1c4b04ed625e8657e25c59e386720cb0f9d735ffc492e98f5b8b2ca3c
1ed3fc9270 Add unit test for tapscript function (Tobin C. Harding)
195615c14d Fix bug in witness stack getters (Tobin C. Harding)
Pull request description:
In #2646 we introduced a bug in the taproot witness stack getter functions, of which we have three:
- `tapscript`
- `taproot_control_block`
- `taproot_annex`
Each returns `Some` if a possible bytes slice is found (with no other guarantees).
Use `taproot_annex` combined with getters from `primitives` to implement the other two getters. This simplifies the code and fixes the bug.
Add an additional getter to `primitives` `Witness::third_from_last`.
Fix: #3598
ACKs for top commit:
sanket1729:
ACK 1ed3fc9270
apoelstra:
ACK 1ed3fc9270c6e46f42f447bf1c63cfa64dafa5e8; successfully ran local tests
Tree-SHA512: fdaa254a05e58a5a8800ac3316ee1dd2e6da83910fa4860156683ba27c594be19549f22cdf7f170142767733240eba2b31f3a31c10626a1ba5c21409e62b9ec5
For the `hashes` crate we would like to make `hex` an optional
dependency. In preparation for doing so do the following:
- Remove the trait bounds from `GeneralHash`
- Split the hex/string stuff out of `impl_bytelike_traits` into a
separate macro.
1db4b723dc Mark function as const (yancy)
Pull request description:
We discussed marking from_vb as const here: https://github.com/rust-bitcoin/rust-bitcoin/issues/3602
However, from what I can tell, map() isn't const and I don't see a tracking issue for changing it. Also, the question mark operator isn't available in const context either, although there is a tracking issue for that: https://github.com/rust-lang/rust/issues/74935. It will be a long while before that makes it into this projects MSRV if/when it lands.
There are some other functions in this module that could use the same re-write to make them const as well it looks like.
ACKs for top commit:
tcharding:
ACK 1db4b723dc
apoelstra:
ACK 1db4b723dc59568c14c76598e7c63c58651ed1cd; successfully ran local tests
Tree-SHA512: 62b612791dd3ce2f6ebf27f586a1262633a46566b9fd3583984171f62209714ad979439345fe86d8ef87d2f78a2cee21d619e2eb3621689faf59d81640e9f77c
The `impl_bytelike_traits` macro is public and it is used in the
`hash_newtype` macro, also public.
Currently if a user calls the `hash_newtype` macro in a crate that
depends on `hashes` without the `serde` feature enabled and with no
`serde` dependency everything works. However if the user then adds a
dependency that happens to enable the `serde` feature in `hashes` their
build will blow up because `serde` code will start getting called from
the original crate's call to `hash_newtype`.
Pull the serde stuff out of `hash_newtype` and provide a macro to
implement it `impl_serde_for_newtype`.
b96501981f feat(units): add kvb constructor to `FeeRate` (Rob N)
Pull request description:
This is partially to check my understanding. I did poorly in physics class as part of my misunderstanding of unit conversions.
The `feefilter` message expresses the fee as satoshis per 1,000 vbyte, see the [p2p handling](1dda1892b6/src/net_processing.cpp (L5331)) which calls [this function](https://github.com/bitcoin/bitcoin/blob/master/src/policy/feerate.h#L63) on the mempool. I presume all that needs to happen to express this in `kwu` is to divide by 4, but ideally a `FeeRate` may be constructed directly from a `feefilter` message using a constructor.
My conversion is (sat / kvb * kvb / 4 kwu)
ACKs for top commit:
sanket1729:
ACK b96501981f.
apoelstra:
ACK b96501981ffef1d32c839cd44861736ce3683964; successfully ran local tests
tcharding:
ACK b96501981f
Tree-SHA512: 5aec75d253a0955bfe0bdec42d2e13b69602da057ec7769126399473799455f8df1691f98f6e8dc1d6338958e05702a846d0a5f00bce6aa9079c2ceb0c54df0f
Add a unit test that verifies we correctly remove the annex when getting
the tapscript from the witness stack.
Unit test written by Casey, pulled out of #3599.
Co-developed-by: Casey Rodarmor <casey@rodarmor.com>
In #2646 we introduced a bug in the taproot witness stack getter
functions, of which we have three:
- `tapscript`
- `taproot_control_block`
- `taproot_annex`
Each returns `Some` if a possible bytes slice is found (with no other
guarantees).
Use `taproot_annex` combined with getters from `primitives` to implement
the other two getters. This simplifies the code and fixes the bug.
Add an additional getter to `primitives` `Witness::third_from_last`.
Fix: #3598
727c519efa Re-export amount from primitives (Tobin C. Harding)
84ede349b0 Run the formatter (Tobin C. Harding)
4865d60258 bitcoin: Improve the re-exports ... again (Tobin C. Harding)
Pull request description:
When we messed with the re-exports recently we failed to notice that the formatter would move the `pub use` blocks around because I only put `rustfmt::skip` on one block, I thought that had the effect of stopping all exports below that block being changed, turns out I was wrong.
Run the formatter then make some minor changes to the re-exports in `bitcoin`, including changing comments that do not make sense with the new layout.
ACKs for top commit:
apoelstra:
ACK 727c519efa771d3ec6a21350e4638b9f1536bfe1; successfully ran local tests
Tree-SHA512: fa9fbadf7efa0a76db6a0a2915fcabeda348c561e4047f9517ca7fba1a2008d5624072b7a9ce6069d71c229a794b4d27c3d96195768e87b3c2c412d611ecbfcf
2ae6a9abc1 Prep the chacha20-poly1305 crate for release (Nick Johnson)
Pull request description:
Bump the patch version to `v0.1.1` to include the no_std support.
Could see this requiring `v0.2.0`, but went with the patch bump.
ACKs for top commit:
apoelstra:
ACK 2ae6a9abc1cb6b0f3edb358ebe5e9a0781051abe; successfully ran local tests; let's do it
tcharding:
ACK 2ae6a9abc1
Tree-SHA512: 880e8390447c238bfd0e9493682622213b91f7f41c8c320fea6af763e00d73fb30bfc545f420b79614a8b33249abbd08bc9dcb7ace9522a68862194240b7258a
We are trying to make it so that what ever crate a user uses they see
the same module/type tree (if the module or type exists).
E.g., one can do either of these.
If they use `bitcoin`:
```
use bitcoin::{
amount, block, fee_rate, weight, merkle_tree, opcodes,
pow, script, sequence, transaction, witness,
};
```
Or if they use `primitives`:
```
use bitcoin_primitives::{amount, block, fee_rate, weight};
```
The above imports were tested and `amount` was found to be missing.
These re-exports are hard to get right. We (I) recently made an attempt
to make our stack of crates easier to navigate using the idea that
exporting a type from the crate it is defined in adds some additional
information without any loss of clarity. Note however that the module
must be re-exported from the "highest" place possible because we at
times add additional functionality as we move up the stack e.g.,
`bitcoin::merkle_tree` has logic in it that `primitives::merkle_tree`
does not. In order to future proof the codebase default to always using
the highest module up the stack even when that module adds no additional
code e.g., re-export `blockdata::fee_rate` as opposed to
`units::fee_rate` in the event that we later add logic to
`bitcoin::blockdata::fee_rate`.
This patch adds an additional re-export: `sequence` (previously missing).