the `blockdata` directory is code organisation thing, all the
types/modules are re-exported from other places. In preparation for, and
to make easier, the `primitives` crate smashing work - remove all
explicit usage of `blockdata`.
Note that the few instances remain as they seem required e.g.,
`pub(in crate::blockdata::script)`
Refactor only, no logic changes.
a0c139d40c Automated update to Github CI to rustc nightly-2024-06-19 (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 a0c139d40c
Tree-SHA512: c2cfc3c9235c50d15ab248c2eb56378300a1684983b6c832de62287c4d1dcb87be45de9dbceaeaa68f176fdbbb7aa1547d91b72636b0db6943ce64563dbae2c3
90b6d6748b merkle_tree: remove some now-redundant code from block.rs (Andrew Poelstra)
48ad5e4789 api changes for "rewrite merkle root calculation" (Andrew Poelstra)
f7ce9bbee8 merkle_node: rewrite algorithm (Andrew Poelstra)
2bc97b22e2 api changes for new calculate_root method (Andrew Poelstra)
8d5cb014ce merkle_tree: introduce MerkleNode trait to better-type merkle tree calculation (Andrew Poelstra)
04353901ba api changes after moving the hashes (Andrew Poelstra)
5d8aa94069 Move merkle_tree hash types (Andrew Poelstra)
Pull request description:
Alternate to https://github.com/rust-bitcoin/rust-bitcoin/pull/2853. Takes patch 1 from that PR.
ACKs for top commit:
Kixunil:
ACK 90b6d6748b
tcharding:
ACK 90b6d6748b
Tree-SHA512: 0e0a3f532e2efb4116f4ebc50719d3a2022c37405b0a22b15f79189b54b623e906e8fdd53997efc611eac5ceaee4c0ef12dab1d5cb595755c0c9e091c0a5904f
e9de541428 Updated workflows/README.md to mention new shellcheck job (Jamil Lambert, PhD)
Pull request description:
Added the new Shellcheck CI job to `./github/workflows/README.md` to fix: #2767
ACKs for top commit:
storopoli:
ACK e9de541428
apoelstra:
ACK e9de541428 pretty-much anything here is fine by me
tcharding:
ACK e9de541428
Tree-SHA512: 805d301a39ec17c159e78c275e28ac0046237e36860987cb70e2d872a29fb5ea65c891adc72ebf2d618ad5686f1548fca2cc139be23386580cd5a115e9fc6faa
Drop recursion, reduce memory usage to be logarithmic in size of tree
rather than linear, and put it all in one function rather than three.
Also make the method an trait method on MerkleNode which makes it a
easier on type inference, by writing e.g. TxMerkleNode::calculate_root.
These changes are nontrivial. They
* Drop the `From` impls from (w)txids to (w)TxMerkleRoots
* Introduce a new trait and bound calculate_root on it...
* ...in such a way that its return value cannot be inferred without
further hints (though in practice this doesn't matter because usually
the return value is immediately assigned to something with a known
type such as a BlockHeader field)
Currently we are defining the two merkle tree hash types in the `block`
module, a better home for them is the `merkle_tree` module.
This is an API breaking change because the types were public in the
`block` module, however the change should/could be unnoticeable to users
if they use the crate level re-export - which is maintained.
e7f33a2a12 Update PushBytes::read_scriptint(x) to x.read_scriptint() (Shing Him Ng)
Pull request description:
Fixes#2849
There were a few other usages of PushBytes::read_scriptint(_) in the tests such as [this](406e3486ab/bitcoin/src/blockdata/script/tests.rs (L315)), but they couldn't be updated as cleanly as the changes in this PR. I wasn't sure if the intention of this issue was to fix those as well, but I can update this PR if needed
ACKs for top commit:
tcharding:
ACK e7f33a2a12
Kixunil:
ACK e7f33a2a12
Tree-SHA512: 4620f4972c40b0bf7333dbe302d1dabc5dbcb39749c734cc297a019d36983757f59659d76ae40b4a6121e51d0bde1e2b7883a0c77536be18927d1cfef53ba579
6014788a74 api: Run just check-api (Tobin C. Harding)
e87a54f617 Enforce script size limit when hashing scripts (Tobin C. Harding)
4686d48ec4 Rename p2wsh script to witness_script (Tobin C. Harding)
9cdb514434 Remove odd script hash test (Tobin C. Harding)
Pull request description:
There are two limits that the Bitcoin network enforces in regard to hashing scripts
- For P2SH the redeem script must be less than 520 bytes (bip 16)
- For P2WSH the witness script must be less than 10,000 bytes (bip 141)
Currently we are only enforcing the p2sh limit when creating an address with `Address::p2sh`.
There are various ways to create addresses from script hashes and if users manually hash a script then use the `ScriptHash` (or `WScritpHash`) our APIs assume the script that was hashed is valid. This means there is the potential for users to get burned by creating addresses that cannot be spent, something we would like to avoid.
- Add fallible constructors to `ScriptHash` and `WScriptHash`
- Add `TryFrom` impls as well to both types
- Remove the `From` impls
Close: #2794
ACKs for top commit:
apoelstra:
ACK 6014788a74
Tree-SHA512: 90c722ee6b63d02ebf39b77bf003f71ec90e745b32831a0d69aa4d6f363624bd6d2c86445f8807ed319d1845d639d2494756a8321207f917e57377d4557c436f
418bb29ccd Automated update to Github CI to rustc nightly-2024-06-16 (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 418bb29ccd
Tree-SHA512: 1dd930ef735dca5cc4af8292623a0d221b475478d67586a4eabef608b399eab0fc589048f84f0263d06e22d021353a85014906f84a0badd13352d6e157063062
There are two limits that the Bitcoin network enforces in regard to
hashing scripts
- For P2SH the redeem script must be less than 520 bytes
- For P2WSH the witness script must be less than 10,000 bytes
Currently we are only enforcing the p2sh limit when creating an address
with `Address::p2sh`.
There are various ways to create addresses from script hashes and if
users manually hash a script then use the `ScriptHash` (or
`WScritpHash`) our APIs assume the script that was hashed is valid. This
means there is the potential for users to get burned by creating
addresses that cannot be spent, something we would like to avoid.
- Add fallible constructors to `ScriptHash` and `WScriptHash`
- Add `TryFrom` impls as well to both types
- Remove the `From` impls
The script in a p2wsh is typically referred to as the witness script not
the redeem script - rename the local test variable to follow suit.
Refactor only, no logic changes.
This test does an odd combination of function calls, its not obvious
what it is supposed to be testing. The `to_p2wsh.is_p2wsh` is already
tested above. The leading `to_p2sh` does not prove anything, one can put
currently pass any script to `to_p2wsh` so this tests nothing.
In preparation for patching the script hashing functionality first
remove this odd test.
1f58476cb4 Run schemars test from extra_tests (Tobin C. Harding)
Pull request description:
We have a mechanism to run additional custom tests by way of the `extra_tests.sh` script in each crate.
Remove the CI job and run the schemars test using `extra_tests.sh`. This patch changes the test coverage because currently the schemars test is only run with a stable toolchain but with this patch applied it runs with stable, MSRV, and nightly.
Fix: #2787
ACKs for top commit:
apoelstra:
ACK 1f58476cb4
Tree-SHA512: 607132890ed08bf75fb544a0e10aeeda5f9c137eb04349f8af5ab28866408d4208cae4688c08645ffca95e7d9568562dbbbfa992382b5d2cb3efeba583d78b1f
3f4eb07769 Add a comment to regression test (Jamil Lambert, PhD)
fc2876ba10 Move use statements to top of module (Jamil Lambert, PhD)
778a44dd64 Refactor merkle block test (Jamil Lambert, PhD)
c4c1252a9e Change encode path (Jamil Lambert, PhD)
Pull request description:
Refactored the `extract_matches_from_merkleblock()` test function following https://github.com/rust-bitcoin/rust-bitcoin/pull/2859#issuecomment-2161710169.
Moved use statements to the top of the test module and changed it to use one level of path instead of importing the function names. e.g. `encode::serialize()` instead of `serialize()`.
Added the missing comment to the `regression_2606()` test. I was not sure where the hex value came from that was used to test that the deserialization fails. The comment was generated by copilot and may need to be edited, it does fit with the error given by deserialize: `OversizedVectorAllocation { requested: 12811880876963004416, max: 4000000 }`.
ACKs for top commit:
tcharding:
ACK 3f4eb07769
apoelstra:
ACK 3f4eb07769 the `prelude::*` is fine since it was already there since #298, but FYI I would not have accepted it today
Tree-SHA512: 203a30eee51ea91051cb10d5d7dd55b560d9d4d785120143c9fb29ea26ec77696124adc9c5bcb8cd736a7d293b897e665958bec5f66626a5c1c95c98b6029e0d
9f01871c11 api: Run just check-api (Tobin C. Harding)
7929b51640 Pass keys by value (Tobin C. Harding)
Pull request description:
We should pass `Copy` types by value not by reference. Pass the key types by value.
This is patch 1 from #2404
ACKs for top commit:
apoelstra:
ACK 9f01871c11 this will annoy some people but I think we should do it
Tree-SHA512: 18afab537edf4ade4dc1c1e5992e50060b8935531f1e3cbe1d3b94b2fcb87aafa39947f342e0e762835bda3b4091dd35b3b74ea79f4dbb3b21660ffd21d1f82e
c81e330e48 Link to std::error::Error (Tobin C. Harding)
Pull request description:
In #2521 I removed the link from `std::error::Error` with the claim that it broke no-std builds. However there are a ton of other places where we link to `std::` types.
I have no idea where the breakage was, I assume it existed and I was sane at the time, CI on this patch will tell us.
Close: #2571
ACKs for top commit:
apoelstra:
ACK c81e330e48
Tree-SHA512: 137fe62eb88359a6439ce5cddaf615704cc72a3df256e66d3730ce429c7b86973d62749c658796e95f9119443da0e286db78a41641b560a5667b076660c9b2b5
18b2788a5a api: Run just check-api (Tobin C. Harding)
6b7d02e5ae Add inherent functions to hashes (Tobin C. Harding)
Pull request description:
Currently we have a trait `Hash` that is required for `Hmac`, `Hkdf`, and other use cases. However, it is unegonomic for users who just want to do a simple hash to have to import the trait.
Add inherent functions to all hash types including those created with the new wrapper type macros.
This patch introduces some duplicate code but we are trying to make progress in the hashes API re-write. We can come back and de-dublicate later.
Includes making `to_byte_array`,`from_byte_array`, `as_byte_array`, and `all_zeros` const where easily possible.
ACKs for top commit:
apoelstra:
ACK 18b2788a5a
Tree-SHA512: 6b7a8d8a8501e981416d767040e5bd9fa8d1134be2ca133b5c53aa55f65c8456dccb63b642e30d0d571ca838c6f9eaeff6527d92a9b4212819a49ce619c4e093
In #2521 I removed the link from `std::error::Error` with the claim that
it broke no-std builds. However there are a ton of other places where we
link to `std::` types.
I have no idea where the breakage was, I assume it existed and I was
sane at the time, CI on this patch will tell us.
Close: #2571
We have a mechanism to run additional custom tests by way of the
`extra_tests.sh` script in each crate.
Remove the CI job and run the schemars test using `extra_tests.sh`. This
patch changes the test coverage because currently the schemars test is
only run with a stable toolchain but with this patch applied it runs
with stable, MSRV, and nightly.
Fix: #2787
024d1fa53b Automated update to Github CI to rustc nightly-2024-06-12 (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 024d1fa53b
Tree-SHA512: e075c3aca81ab3cddbac470f17d48acf39483e664341894203f042a01cec12534efd013c070c6985c864687ef8575f2df31d2eb8af33dea952bb6676f667640a
Currently we have a trait `Hash` that is required for `Hmac`, `Hkdf`,
and other use cases. However, it is unegonomic for users who just want
to do a simple hash to have to import the trait.
Add inherent functions to all hash types including those created with
the new wrapper type macros.
This patch introduces some duplicate code but we are trying to make
progress in the hashes API re-write. We can come back and de-dublicate
later.
Includes making `to_byte_array`,`from_byte_array`, `as_byte_array`, and
`all_zeros` const where easily possible.
8d256b4e79 Moved doctest to unit test (Jamil Lambert, PhD)
Pull request description:
Moved the doctest from the private module to a unit test to fix: #2840
ACKs for top commit:
apoelstra:
ACK 8d256b4e79
tcharding:
ACK 8d256b4e79
Tree-SHA512: 233961288579381cb5c51235331c78c9980046f0317a2062cb108a2e604adde322e2dd834e724d1fe05f54f92721fe6c9f5344dcf930f99aa3756a698ddf9732
fcf27a62a2 update api (Divyansh Gupta)
531aba0cf1 make `difficulty_float` general to all network (Divyansh Gupta)
Pull request description:
Fix#2783
ACKs for top commit:
tcharding:
ACK fcf27a62a2
apoelstra:
ACK fcf27a62a2
Tree-SHA512: aec9648b057677b89f4f397e1fa703ed6496436bd6e1e8052d5b7f52ef52c95b286babd425d97fe7d4f903cd0a3d68ae589d8fb255fc68f423714918cd485873
091d614aad bitcoin: Remove "std" feature from examples (Tobin C. Harding)
Pull request description:
The "rand-std" feature enables "std" but we use it in examples still. FTR I added this a while ago thinking the explicitness was clearer but in hindsight I think that was wrong and that it makes usage of our features _less_ clear.
No logic changes.
(Pulled out of #2756.)
ACKs for top commit:
storopoli:
ACK 091d614aad
apoelstra:
ACK 091d614aad
Tree-SHA512: d72eec37e3a434a1f850cb4257529fc18540cb5075bc7d3bd494cba59b08404c6c86223361a3c3a3de8d50a1168b656ff1123d28f7d2dcdf05c404caff716b1a
2db88a62fd Update bitcoinconsensus version (Jamil Lambert, PhD)
Pull request description:
Updated bitcoinconsensus version to 0.106.0+26.0 in bitcoin/Cargo.toml
The new version supports taproot and has a new parameter for `spent_outputs` in the `verify()` and `verify_with_flags()` functions.
The validation module was changed to keep the existing functionality by adding `None` as the `spent_outputs` and the flag `VERIFY_ALL_PRE_TAPROOT`.
This method does not add taproot features to the verify functions.
ACKs for top commit:
tcharding:
ACK 2db88a62fd
apoelstra:
ACK 2db88a62fd agreed, this is a good step forward
Tree-SHA512: b6cef395e065cfe859a7896ee2deb2f2d255051566751c10d06092064f6523338ca912aad81b789d6ca94e6f6164f2eef874ad6086c6ebe1384ce520d6eba366
8aa893ebd0 Remove repetition from sha256t_hash_newtype macro (Tobin C. Harding)
Pull request description:
The `sha256t_hash_newtype` macro is hard to reason about because we allow repetition so which tag goes with which type is slightly obscure.
Remove repetition and call the macro three times.
Internal change in `bitcoin`, API change in `hashes`.
Fix#2811
ACKs for top commit:
apoelstra:
ACK 8aa893ebd0 nice, small diff
Tree-SHA512: b38e7c307ac7288b4a5c1c3170ad6aa54c62bd3198922ec8bb091867b230bb9149f7dc996766fc8fa20a1af18b318c475b3e83e2689d322b7f4af0d5cb588e50
ce1db3ea26 hashes: Move non-trait functions (Tobin C. Harding)
Pull request description:
The `hash_trait_impls` macro currently adds an impl block for `Hash` - this is not what the docs say since and `impl Hash` block is nothing to do with traits.
Move the impl block and add a duplicate of the functions to the `sha256t::Hash` type.
This is a refactor, no API or logic changes. Note that wrapper types currently do net get these functions - that will be discussed/implemented separately.
ACKs for top commit:
apoelstra:
ACK ce1db3ea26 Ok, this one is easy :)
Tree-SHA512: c25bc2bcfdae8130bbcb9d212701504e15a8429630ae4ce924dcf1c54bb10e122aa67d1abed8bb338e4844de715816dc49ca4de7bf16033f293817c398717756
Tobin C. Harding
Update Nightly Rustc Bot
Andrew Poelstra
Jamil Lambert, PhD
Shing Him Ng
Divyansh Gupta