4652ce20ed API changes for "delete `all_zeros`" (Andrew Poelstra)
8869f35a69 hashes: drop the `all_zeros` method on arbitrary hashes (Andrew Poelstra)
9f8797f486 API changes for constification of hash constructors (Andrew Poelstra)
154e91af8c hashes: constify a bunch of constructors (Andrew Poelstra)
c155cbf8b2 hashes: use workaround to get constfns on tagged hashes with MSRV (Andrew Poelstra)
Pull request description:
I think these changes are both uncontroversial but they have fairly large diffs so I am PRing them together before making more invasive changes.
ACKs for top commit:
tcharding:
ACK 4652ce20ed
Tree-SHA512: 4560fa397deab50448598894b9364f9d8f8b48169901a84db6a44168cdba795ab69b48ad2cac61caebcee5e227a03271335b405cf5514265290a4d1f2fdf52a2
76f97677f7 Automated update to Github CI to rustc nightly-2024-06-23 (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 76f97677f7
Tree-SHA512: 153743100b41c64542ac1d576d6ef0675e5bc4783b50c76c7231f3b7e00d38bb7b1e965f4cf53820dd1bca40ebbaf2e41caab7363b6cc150afa337fcd106fe21
aebf216619 Use 100 column width for rustdoc (Tobin C. Harding)
c71ae9ac16 Move PushBytes::read_scriptint (Tobin C. Harding)
Pull request description:
The `push_bytes` module has a `private` module that exists solely to protect the invariant on the `PushBytes` inner byte slice. There is a `PushBytes` impl block outside the private module for functions that can not and do not violate the length invariant.
Recently we move the `read_scriptint` method to be on the `PushBytes` but we put it inside the `private` module, since the method only reads off of the slice it cannot invalidate the invariant and does not need to be inside the `private` module.
Move the `read_scriptint` method outside of the `private` module to keep that module as small as possible, helping with its stated aim of being the only place that requires auditing.
Patch 2 is whitespace only.
ACKs for top commit:
Kixunil:
ACK aebf216619
apoelstra:
ACK aebf216619
Tree-SHA512: f41eb691e7118a74767a2582a637a176a46e4a8d15259f1c1f3cef67bc57dd8dce4974407c5a5e31acd0b4665f722b20acad4e381747c9d496f18158a0ef9def
72655607b6 api: Run just check-api (Tobin C. Harding)
e8250cd96a Remove InvalidInternalKey variant from TaprootBuilderError (Tobin C. Harding)
Pull request description:
This variant is unused, remove it.
Done as part of #2883.
ACKs for top commit:
Kixunil:
ACK 72655607b6
apoelstra:
ACK 72655607b6
Tree-SHA512: 5cd27cacebcf078afdf1ceba4e3d51e78f20ee4883000b766efb7a246fd7a166240038a2e7d27249a22049c3258673a393ff2fd62cb4b27a2cade04b28ef2ac9
433fd6bf7e api: Run just check-api (Tobin C. Harding)
8fd583b069 Pass hash types by value (Tobin C. Harding)
Pull request description:
We should pass `Copy` types by value not by reference. Pass the hash types by value.
Second step in the pass-copy-types-by-value work, pulled out of #2404.
ACKs for top commit:
apoelstra:
ACK 433fd6bf7e
Kixunil:
ACK 433fd6bf7e
Tree-SHA512: 999d12f60550cacc4ae19b4cbf505b25c1eed803820f22b1a706e9f95da1b7e7b422f393f4115d579927c0c476cd504036a39b3cdc06a1d6befbcff5513f7433
39df0a9fbe update api (Divyansh Gupta)
3a5f2932a4 create constants for ChildNumber enum (Divyansh Gupta)
Pull request description:
this aims to fix#2750
ACKs for top commit:
tcharding:
ACK 39df0a9fbe
Tree-SHA512: e1c38568facd2b9aa55b1b1ec0d5d5f68ff38ca3fe68962bc316c060a062299935aa51bcfc1c255a7f5c9ad97435cab22e2c160d3fd3f52a46f6b5cbb7d5743f
Manually implement it for Wtxid, Txid and BlockHash, where the all-zero
"hash" has a consensus meaning. But in general we should not be
implementing this method unless we have a good reason to do so. It can
be emulated or implemeted in terms of from_byte_array.
The use of Wtxid::all_zeros is obscure and specific enough that I am
tempted to drop it. But for txid and blockhash, the 0 hash appears in
actual blockdata and we should keep it.
All other uses of all_zeros were either in test code or in places where
the specific hash was not important and [u8; 32] was a more appropriate
type.
As you can see from the - lines in the API diff, there is no reduction
in API surface (we just remove the T:Tag bound from the sha256t::Tag
type, which is not strictly necessary but maybe we would prefer to keep).
We use 100 column width for rustdoc in this project, while not a super
hard rule the docs on `read_scriptint` are long, using the 100 column
width reduces the line count a reasonable amount.
No text changes, only whitespace.
The `push_bytes` module has a `private` module that exists solely to
protect the invariant on the `PushBytes` inner byte slice. There is a
`PushBytes` impl block outside the private module for functions that can
not and do not violate the length invariant.
Recently we move the `read_scriptint` method to be on the `PushBytes`
but we put it inside the `private` module, since the method only reads
off of the slice it cannot invalidate the invariant and does not need
to be inside the `private` module.
Move the `read_scriptint` method outside of the `private` module to keep
that module as small as possible, helping with its stated aim of being
the only place that requires auditing.
7e3b2ebb74 CI: Pin cargo-public-api version (Tobin C. Harding)
Pull request description:
So that updates don't break our CI setup used a specific version of `cargo-public-api`, this means we will have to update it manually periodically though.
For now, since the latest release just broke us, use the release from a month ago.
ACKs for top commit:
Kixunil:
ACK 7e3b2ebb74
apoelstra:
ACK 7e3b2ebb74
Tree-SHA512: 9266f1c193395e25b1fc7945b2ed10fa7418c73f483ee3c6ad6c713b41a9eb9a01e1e23a58df5720e53811d470ac7465cffc235471236447f9c985dea5af21bb
a42bcdc22e Remove usage of blockdata from paths (Tobin C. Harding)
Pull request description:
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.
Done as part of #2883
ACKs for top commit:
apoelstra:
ACK a42bcdc22e lgtm! I consider this "trivial" and will one-ACK merge it
Tree-SHA512: 310605e5203cf04aaeb91fe5512677b8f1438b183916686ba2cdc41ffdc18af7a0676206724e8a14c50ce6ed8faa9d48c69a2d5149eb1f56ae9c5f276fc5200f
So that updates don't break our CI setup used a specific version of
`cargo-public-api`, this means we will have to update it manually
periodically though.
For now, since the latest release just broke us, use the release from a
month ago.
16e4d22693 clarify the meaning of Height & Time based locktime. (Divyansh Gupta)
Pull request description:
this pr aims to fix : #2697
ACKs for top commit:
apoelstra:
ACK 16e4d22693
tcharding:
ACK 16e4d22693
Tree-SHA512: 55757d7e593cb284aff7040cf3298931c7f3d8e9e36d7328bd748a39be743e5c4202c55505add0219b2766d35d1660affc5ed4a7b9480b3a3bfb89982fe3970a
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.
Andrew Poelstra
Tobin C. Harding
Update Nightly Rustc Bot
Jose Storopoli
Divyansh Gupta
Jamil Lambert, PhD
Shing Him Ng