894f82e7cc Add a condition for parsing zero from string when not denominated. (yancy)
Pull request description:
closes https://github.com/rust-bitcoin/rust-bitcoin/issues/3307
ACKs for top commit:
Kixunil:
ACK 894f82e7cc
tcharding:
ACK 894f82e7cc
apoelstra:
ACK 894f82e7cc9eb459a297d43e82734621e0824610; successfully ran local tests.
Tree-SHA512: 6d32847c74ccedc3355d615838e2dd1e08add29cc47ff69d9ef22683eda93049d41131dd0c992c8d7be3a018f5438856c415a3d2f3cb9de9c986534b268ee386
8f2f4cbb3c Re-order optional dependencies (Tobin C. Harding)
95f2a8dab6 Do not access ScriptBuf inner from builder (Tobin C. Harding)
900af453ff Stop accessing inner ScriptBuf field when encoding (Tobin C. Harding)
8b82363d97 Use Script::as_bytes instead of inner when indexing (Tobin C. Harding)
b0675a4a4f Use Script::len instead of inner field (Tobin C. Harding)
374c6118dc Deprecate Script::fmt_asm and to_asm_str (Tobin C. Harding)
Pull request description:
Move the `Script` and `ScriptBuf` types to `primitives`. There were still a few preparations required, things we had missed while creating the extension traits.
Note also please, in the last patch, we enable `hex` from the `serder` feature. This is not the final state we want but like we did for `alloc` it is a step to reduce the size of the diff.
ACKs for top commit:
Kixunil:
ACK 8f2f4cbb3c
apoelstra:
ACK 8f2f4cbb3c successfully ran local tests
Tree-SHA512: 62a5f3c253ecb54d95c37fdc7eb955f3952909dc3bca20444b85c44665f54d5a0c48daf729bed0dd60ff3e9571b41deed039984c8b757b075ac6e136cacd17d7
6c0aaa0915 hashes: Put test function in a module (Tobin C. Harding)
Pull request description:
With a recent nightly toolchain `clippy` gives us an error:
error: missing documentation for a constant
I'm not sure why the error is emitted but wrapping the function in a `tests` module as is standard practice clears the error.
ACKs for top commit:
Kixunil:
ACK 6c0aaa0915
apoelstra:
ACK 6c0aaa0915 successfully ran local tests
Tree-SHA512: f6b72c439d3fb34944b58565afaaeeea9e9420d4031091d0215f8c514b64f4ea593f155363fd173f022739422b504e3b2215ce60b35d55b4fd8a3554c5a84f3c
0403e52ce3 Move the transaction hash types over to primitives (Tobin C. Harding)
7e454d756d Define extension traits for txid types (Tobin C. Harding)
832b726d03 Stop using all_zeros (Tobin C. Harding)
d69c241b5c Improve docs on associated consts (Tobin C. Harding)
68c9e28165 Do not use private constructor for txid hash types (Tobin C. Harding)
98328b5a7b Use as_byte_array to encode hash type (Tobin C. Harding)
Pull request description:
Move the `Txid` and `Wtxid` hash wrapper types over to `primitives`. This introduces to `primitves` an unconditional dependency on `hashes`.
ACKs for top commit:
Kixunil:
ACK 0403e52ce3
apoelstra:
ACK 0403e52ce3 successfully ran local tests
Tree-SHA512: d14fa95bc12c2399d30d4d640b5a3fce625d51adf587a8037158f7d7e7b6288170b2d4418ca2cb68f612086ea0bdd0fae3b577f84f0d60627072fdb2217a6531
ff88fdf544 CI: Run the nightly update job at 5 past midnight (Tobin C. Harding)
Pull request description:
Currently we run the job at midnight here and in `sepc256k1`, this led recently to one using the nightly toolchain from the 10th of Sep and the other using the toolchain from 11th of Sep.
Update to run at 5 past so this doesn't happen again.
ACKs for top commit:
Kixunil:
ACK ff88fdf544
apoelstra:
ACK ff88fdf544 successfully ran local tests
Tree-SHA512: 063e062f5c7edaf8ef6622365cdcaec1e5e6f2f21d076ffa07ded80774c28dad28035f56515b7b4f21b36159c6d1d219ffd01169caa94a5ea1301e62e0341262
e064686397 Deprecate OutPoint::new constructor (Tobin C. Harding)
Pull request description:
The `OutPoint` type has two public fields, providing a `new` constructor that just sets these two fields adds no value.
Done after discussion in #3340 as part of `primitives` work.
ACKs for top commit:
Kixunil:
ACK e064686397
apoelstra:
ACK e064686397 successfully ran local tests
Tree-SHA512: d80cac85093946b3678883f4e3ad7fd2052d858dbd3fab4127916b0eb6153999d30827c84ccf8c6e4412fbc3842bd127a2e4d3ca0248d2307d0bca467a2555ce
f811e0adb6 Stop using deprecated OutPoint functions (Tobin C. Harding)
Pull request description:
Either our CI is failing us or `deprecated` does not work as expected, either way we should not be using the `OutPoint::null()` or `is_null` functions any more because we deprecated them already.
ACKs for top commit:
Kixunil:
ACK f811e0adb6
apoelstra:
ACK f811e0adb6 successfully ran local tests
Tree-SHA512: 64e75601ce8062da78bbd9aea97792e35c728aa31255348413fe9b57ee585974370d3f59b5467d7e4a65dc05d2718aea18bb381e128c8410fbfde4a851006416
With a recent nightly toolchain `clippy` gives us an error:
error: missing documentation for a constant
I'm not sure why the error is emitted but wrapping the function in a
`tests` module as is standard practice clears the error.
Currently we run the job at midnight here and in `sepc256k1`, this led
recently to one using the nightly toolchain from the 10th of Sep and the
other using the toolchain from 11th of Sep.
Update to run at 5 past so this doesn't happen again.
514cc5500f internals: Fix lint warnings on macro (Tobin C. Harding)
Pull request description:
The new `impl_to_hex_from_lower_hex` macro causes build warnings when `internals` is built without the `alloc` feature.
There are two macro implementations depending on feature gates, duplicate the rustdocs from the other one.
While we are at it reduce the lines of code for the empty case.
ACKs for top commit:
Kixunil:
ACK 514cc5500f
apoelstra:
ACK 514cc5500f successfully ran local tests
Tree-SHA512: f6a8929f53be77fed3703858b757fc94bd986dccde417a1cbd8edda67e08f4ff4dc76d5eff7f09a0482a05c9ea330cc45e970f3549fbd0b5f35683dd0243e2a1
The optional dependencies are ordered and separated by whitspace in a
manner that may not be obvious (or even have a reason).
Some of this is because since use of `?` deps changed name.
Put all the optional deps together in alphabetic order.
The `Builder` is staying in `bitcoin` while the `ScriptBuf` is moving to
`primitives`, so we cannot access the inner field of `ScriptBuf`.
Use the new `as_byte_vec` hack to mutate the inner `ScriptBuf` field.
In preparation for moving the `ScriptBuf` type to `primitives` stop
accessing the inner field when encoding/decoding, use `as_script`
and `from_bytes` instead.
In preparation for moving the `Script` type to `primitives` stop
accessing the inner field before doing slice operations, use `as_bytes`
to first get at the slice.
The `Script::fmt_asm` function is a legacy from days yore before
`Display` printed asm. We no longer need it.
Deprecate `Script::fmt_asm` and use the private `bytes_to_asm_fmt` or
`Display` impls.
Use the `define_extension_trait` macro to define two extension traits
for the two txid types. Each trait holds the deprecated `all_zeros`
function. There are no users of this trait in the code base.
Recently we deprecated the `all_zeros` functions on `Wtxid` and
`Txid` but for some reason our usage of them is not triggering a lint
warning.
Note please that this changes logic slightly, for example by using an
array of `0xFF` bytes instead of all zeros. Done in an effort to make it
even more obvious that the value is a dummy value and not mix it up with
the all zeros being used for coinbase thing.
In #3308 we added associated consts to the `Txid`, `Wtxid`, and
`OutPoint` types. During review and afterwards we realised the docs
could do with improving. Since we now want to move the types we should
do this first.
Close: #3331
Instead of accessing the inner type of a hash wrapper type when
consensus encoding we can call `as_byte_array()`.
Done in preparation for moving `Txid` and `Wtxid` to `primitives`.
Internal change only.
The new `impl_to_hex_from_lower_hex` macro causes build warnings when
`internals` is built without the `alloc` feature.
There are two macro implementations depending on feature gates,
improve the docs and duplicate them ont both macro definitions.
bd8ad1f5e2 Add basic `miri` checks (Martin Habovstiak)
fb5971cc2b Fix UB in `siphash24` (Martin Habovstiak)
Pull request description:
We have a bit of `unsafe` code in the crates which should really be checked with `miri`. Thus this adds a basic CI check that automatically determines which crates need `miri` checking and checks them. It also makes sure to enable all target features so that SIMD code can be checked as well.
This doesn't try to do anything fancy with maintainer tools or run task for now, since I just want to test the basic idea.
Closes#3192
ACKs for top commit:
storopoli:
ACK bd8ad1f5e2
tcharding:
ACK bd8ad1f5e2
sanket1729:
ACK bd8ad1f5e2
apoelstra:
ACK bd8ad1f5e2 successfully ran local tests; wow, good find!
Tree-SHA512: a0d33c7851d6d6b288ca8cc1a902f187814dd82e3528c6f8169fdc0ba71991b99451276aaba5e3b6cde6029e09158063d65e48a71d1e01ee20302b9f653584ef
d65de7c7de Introduce and use new compact_size module (Tobin C. Harding)
Pull request description:
We would like to move the witness module to `primitives` but there is a bunch of usage of `VarInt`.
Introduce a module that does the encoding and decoding instead, note that while the functionality is internal decoding returns an error which may one day end up in the public API. So put the module in `primitives` and make it public.
Adds the module to `primitives`, adds a public `MAX_ENCODABLE_SIZE` variable that is commented with an issue link.
https://github.com/rust-bitcoin/rust-bitcoin/issues/3264
ACKs for top commit:
apoelstra:
ACK d65de7c7de successfully ran local tests
Kixunil:
ACK d65de7c7de
Tree-SHA512: d9483c29b2b324e27460564a23f4639dde4037e6e773f4356216b02ebdea893a6361c342002b8e93a54de47b71ac69369431554f8cd0a2522fc451bf8493c81c
Either our CI is failing us or `deprecated` does not work as expected,
either way we should not be using the `OutPoint::null()` or `is_null`
functions any more because we deprecated them already.
733505148c Add tests for witness_program (Shing Him Ng)
Pull request description:
Add tests for witness_program
ACKs for top commit:
tcharding:
ACK 733505148c
Kixunil:
ACK 733505148c
apoelstra:
ACK 733505148c successfully ran local tests
Tree-SHA512: c1ee82edf22c7b39bc110d03836ba5ebfa785a63185a75c1a61781180ff907c1ea0c491c963629450f360152766845dacedccc6cf56bc3d8c607e66281427dd3
ae93e226e3 Remove hashes io feature (Tobin C. Harding)
Pull request description:
Currently we only get `std::io::Write` impls when the `bitcoin-io` dependency is used. This is overly restrictive, it would be nice to have `std::io::Write` imlps even without the `bitcoin-io` dependency.
Copy the logic out of the `bitcoin_io::impl_write` macro into `hashes` but feature gate it differently.
Call the new macro inside `hash_type` (and in `hmac`), remove the `impls` module, and move the tests to the integration test directory.
Remove the `io` feature from `hashes`, now if users enable `std` they get `std::io::Write` impls and if they enable `bitcoin-io` they get `bitcoin_io::Write` impls as well.
ACKs for top commit:
Kixunil:
ACK ae93e226e3
apoelstra:
ACK ae93e226e3 successfully ran local tests
Tree-SHA512: d47c9c060750e8a024c46cbf7afe8d0d1245fa1f5e575f36b3a11e2460d3620ad9def1a6331dafe77d46affc99b043ec9679e619ce8ddfa32436a5826ece09e4
fe46225ed0 Allow unused imports when running bench code (Tobin C. Harding)
eb67e873e0 Allow unused variables in release mode (Tobin C. Harding)
Pull request description:
Two patches to clear the million warnings when running the bench code.
ACKs for top commit:
apoelstra:
ACK fe46225ed0 successfully ran local tests; though in the first commit you could also use `cfg_attr` FWIW
Kixunil:
ACK fe46225ed0
Tree-SHA512: 3f705e0441d8c0e41e9ceb5473572810ff2513f7e5531c1b7889418a3a85ac8622e50e271c7a3b5c386fb3f5629b85d4bd79739c4a02b51d58da86890721d8d2
0f897f80a5 Re-write (and re-name) read_uint_iter (Tobin C. Harding)
Pull request description:
The `UintError` type (returned by `read_uint_iter`) is not that useful because one variant is unreachable. Re-write the function by doing:n
- Re-write the function to reduce the error cases returned.
- Re-name it to `read_push_data_len`
- Move it to `internals`
- Use `PushDataLenLen` enum instead of an int parameter
ACKs for top commit:
apoelstra:
ACK 0f897f80a5 successfully ran local tests; lol so much better than the old code
Kixunil:
ACK 0f897f80a5
Tree-SHA512: 095017a32c2d5bb2268cb1a059d0022e122faf8b41295f14970e7968374dd1c35c3b95357aba5aabaa17843439aebc237000009015ea9b8bc58ab1b337e8e1bc
ea2efc155e Add coinbase associated consts (Tobin C. Harding)
Pull request description:
Currently we have `all_zeros` functions and `null` functions but we can do better.
Add associated consts and improve the names to better describe what these dummy zero hashes are used for.
Deprecate related functions.
ACKs for top commit:
Kixunil:
ACK ea2efc155e
apoelstra:
ACK ea2efc155e successfully ran local tests
Tree-SHA512: bc7e840622a558bc46798e3606452ad24c16b7d23e7fe7a68fdf8a719326eb9d6d872ec1647620506f1de76b8086ae36cce0e1399e55e50bbd794efb8b4dda47
e788d5659c Automated update to Github CI to cargo-semver-checks version-0.35.0 (Update cargo-semver-checks Bot)
Pull request description:
Automated update to Github CI workflow `semver-checks.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
tcharding:
ACK e788d5659c
Tree-SHA512: 797bede055af6dcce87a30105c63366c1e96456bf5bdf0da234efd1d32694bf68e5835e7fcede751bd5a4e9f8d1b090df7e81a8a299732ace78af60e82d9aa3a
Currently we only get `std::io::Write` impls when the `bitcoin-io`
dependency is used. This is overly restrictive, it would be nice to have
`std::io::Write` imlps even without the `bitcoin-io` dependency.
Copy the logic out of the `bitcoin_io::impl_write` macro into `hashes`
but feature gate it differently.
Call the new macro inside `hash_type` (and in `hmac`), remove the
`impls` module, and move the tests to the integration test directory.
Remove the `io` feature from `hashes`, now if users enable `std` they
get `std::io::Write` impls and if they enable `bitcoin-io` they get
`bitcoin_io::Write` impls as well.
The `UintError` type (returned by `read_uint_iter`) is not that useful
because one variant is unreachable. Re-write the function by doing:n
- Re-write the function to reduce the error cases returned.
- Re-name it to `read_push_data_len`
- Move it to `internals`
- Use `PushDataLenLen` enum instead of an int parameter
We have a bit of `unsafe` code in the crates which should really be
checked with `miri`. Thus this adds a basic CI check that automatically
determines which crates need `miri` checking and checks them. It also
makes sure to enable all target features so that SIMD code can be
checked as well.
eda87517c0 Update documentation to indicate that the Display implementation in Amount is unstable (Shing Him Ng)
Pull request description:
From [this comment](https://github.com/rust-bitcoin/rust-bitcoin/issues/2954#issuecomment-2325679334) in #2954, update the documentation to indicate that the Display implementation in Amount is unstable
ACKs for top commit:
Kixunil:
ACK eda87517c0
apoelstra:
ACK eda87517c0 successfully ran local tests
Tree-SHA512: de761b41696e1f43b1a823b0d8a62ce82ec3247e260b7f2b0253f176224e47da64542889c86b9150a9d1a4e29409008b45882cb16c954cbe601d3c1a0093e3cf
The code in `siphash24` was obtaining the pointer in buffer at offset by
accessing an element at that offset instead of accessing a range or
simply computing the offset of the pointer from the start. This is UB
because one canot access past `T` even if the allocation is known to be
large enough. This change fixes it by using a range and also replaces
complicated code with simpler use of `from_le_bytes`.
It's quite likely that this can be improved further, possibly even
removing the `unsafe` without speed penalty but it's a larger task
that's not a priority right now.
d72f730211 hashes: Use $crate in internal macros (Tobin C. Harding)
Pull request description:
These are only called from within the crate but it is still more correct to use `$crate` and saves this from biting us later if we copy the code someplace else.
Internal change only.
ACKs for top commit:
Kixunil:
ACK d72f730211
apoelstra:
ACK d72f730211 successfully ran local tests
Tree-SHA512: d278643c3fbeb28ca377ebf59958054dd2893c46b48469e03a8c7517c5b0b33271de061ae662c400d45962724fe4d13cada41fd5b839a1ff784521ac69c9db72
c00afe8d52 Change MessageSignatureError to secp256k1::Error (Jamil Lambert, PhD)
a20d0bc4eb Deprecate `from_slice()` in sha256.rs (Jamil Lambert, PhD)
089043546f Deprecate `from_slice` methods in favor of arrays (Jamil Lambert, PhD)
Pull request description:
As brought up in issue #3102 support for Rust arrays is now much better so slice-accepting methods that require a fixed length can be replaced with a method that accepts an array.
`from_slice()` methods that require a fixed length have been deprecated and where needed a `from_byte_array()` method created that accepts an array.
There are still `from_slice` methods that rely on changes to external crates before they can be changed to arrays.
ACKs for top commit:
apoelstra:
ACK c00afe8d52 successfully ran local tests
tcharding:
ACK c00afe8d52
Kixunil:
ACK c00afe8d52
Tree-SHA512: c505b78d5ca57e7e1004df6761ac6760d5d9b63c93edc6ac1f9adf047bc67011883575f835b06f6d35d7f6c2b6a4c6c7f0a82a3f0e293bfb4ef58123b75d3809
333c8ab297 Add additional docs to Witness (Tobin C. Harding)
Pull request description:
The `Witness` struct is non-trivial, in particular it is not immediately obvious where and when the compact size encode value for each witness element is stored.
Make an effort to improve the docs on `Witness` in relation to the compact size encoded length of each witness element.
ACKs for top commit:
apoelstra:
ACK 333c8ab297 successfully ran local tests
Kixunil:
ACK 333c8ab297
Tree-SHA512: 1c61a9ad071c035d5ad2e54446120d29ebf8cc4a779c96f04eda825890687dcbd53accc17522f57ef4ffb226eb1d85c6a3a115f27bebcfc7ad3c677033a8a414
yancy
merge-script
Tobin C. Harding
dependabot[bot]
Shing Him Ng
Martin Habovstiak
Fmt Bot