In the next commits we are going to stop exposing the ability to hash
arbitrary data into wrapped hash types like Txid etc. In preparation for
this, stop using these methods internally.
This makes our internal code a little bit uglier and less DRY. An
alternative approach would be to implement the from_engine and engine
methods, but privately (and maybe having a macro to provide this). But I
think this approach is more straightforward.
The one exception is for the Taproot hashes, which are tagged hashes and
currently do not have their own engine type. I will address these in a
later PR because this one is already too big.
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
We have various different column widths being used in a single rustdoc
block, since we favour 100 for comments around here use it.
No text changes, whitespace only.
We should pass `Copy` types by value not by reference.
Currently this is not done in secp, but lets do it here in bitcoin.
Pass by value:
- `SerializedSignature`
- bitcoin sigs
- secp sigs
- secp `Message`
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.
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.
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
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.
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.
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
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.
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
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
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
Updated bitcoinconsensus version to 0.106.0+26.0.
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`.
ce585dc529 api: Run just check-api (Liu-Cheng Xu)
61565957ad Add API for extracting the inner payload of RawNetworkMessage (Liu-Cheng Xu)
Pull request description:
I'd like to take out the `payload` of `RawNetworkMessage` and then send it to the actual network message processor, but find there is no way to do it. This commit adds such an API to expose all the inner parts (UPD: so that I don't have to do an unnecessary clone to obtain the owned value of `payload`).
ACKs for top commit:
apoelstra:
ACK ce585dc529
tcharding:
ACK ce585dc529
Tree-SHA512: 89c5f1361a8c2d0ecf928325e9a37b26d39cf32c3a023b78742555a5a1e59e1579522e942ff6f48b07b2d3b81b48c5e8182fa6e88b57fc9c33ce13e313ba2982
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`.
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.
76826313a1 generate Network <-> Magic From and TryFrom with a macro (Antoni Spaanderman)
Pull request description:
Removes possible errors when a network is added to the enum (expressed with the comment `// Note: new network entries must explicitly be matched in 'try_from' below.`)
ACKs for top commit:
Kixunil:
ACK 76826313a1
apoelstra:
ACK 76826313a1
Tree-SHA512: 37aaf4b9021204c24e3dc405e666f3dea8c4f8e478b26892dd962a49c988904ba02c3dee7cec1ad78d4e4bb9ba9565ec4574d0e169dd215d26b010f1c4dd3d0b
I'd like to take out the `payload` of RawNetworkMessage and
then send it to the actual network message processor, but
finds there is no way to do it. This commit adds such an API
to expose the owned value of inner `payload`.
Add a unit test that fails if put before the "pow: Fix off-by-one error"
patch. Tests that we can correctly parse a 32 character long hex string
into a `U256`.
d6ef16af30 update api (Divyansh Gupta)
a336ec0dda refactor(script): move `read_scriptint` to `PushBytes` & create `read_int` function (Divyansh Gupta)
Pull request description:
* Moved `read_scriptint` method to `Push_Bytes` struct
* Created `Instruction::read_int` method which acts as a wrappper around this function.
Done as part of #1547
ACKs for top commit:
apoelstra:
ACK d6ef16af30
tcharding:
ACK d6ef16af30
Tree-SHA512: e33df8adcb1c23351da303f6bad1ea4a8eae30e65943d230ae886183a01f970aecd0c8c8fd3a6c337cfe6dde1b7590778d88c308415e393f137065ef7da4b29c
Some of our CI shell scripts are meant only to be sourced and not
run directly however they include an initial shebang line, implying that
they should be run.
Remove the shebang line from `crates.sh` and the various `test_vars.sh`
scripts. Add a `shellcheck` directive to inhibit the no-shebang warning.
Fix: #2764
We just added to now types that are thin wrappers around `u32`s for
block heights and intervals.
Add `Encodable` and `Decodable` impls and use the new types. While we
are at it re-export the types from the crate root so users don't have to
dig into the `units` crate.
9e4b092fce psbt: Use macro instead of function (Tobin C. Harding)
Pull request description:
We have a private function that makes use of the `Hash` trait to generically hash map entries. This usage makes patching the `hashes` module difficult. We can achieve the same thing by using a macro and passing in the concrete type.
This is an internal change, no effect on logic or public API.
ACKs for top commit:
apoelstra:
ACK 9e4b092fce
Tree-SHA512: 8b788fa91d21bbae556c746c2e55e6e9395e022bedf13193555ef7482109b6ef5032b233c5f37543a31ebda49d9b4761c161ca0db501472047eb661a48e944b7
6ba7758b30 Improve array macros (Tobin C. Harding)
Pull request description:
Currently we have two macros used when creating array wrapper types, one is in `internals` and the other in `bitcoin::internal_macros`. It is not immediately obvious what is what and why there are two.
Improve the macros by:
- Move the inherent functions to `impl_array_newtype`
- Use `*_byte_array` for the names instead of `*_bytes`
- Re-name the other macro to match what it now does
ACKs for top commit:
apoelstra:
ACK 6ba7758b30
Tree-SHA512: 36ed0fae0d28f24d29287062eb05bbc1e9e8b565f4ff41fd893503a25404ed8e185a34d75e398a8a660923ffda3b832b6157011598d5a75a5c4aafdffc74af2a
eda61ddfef Deprecate to_vec in favour of to_bytes (Tobin C. Harding)
Pull request description:
Currently we have to method names for the same thing "copy this object into a vector". The library is easier to use if we are uniform and just use one.
Elect to use `to_bytes`, for context see discussion in PR #2585.
ACKs for top commit:
apoelstra:
ACK eda61ddfef Nice. IMO we should start deprecating stuff for two releases rather than one, so that people have a year to update.
Tree-SHA512: 0aadd1258a07bfa53806f19a3c41af8d3b1132aa42e7a2015a59c58c4309d7a9b50b86d076c181ce5870ba5acd989feec32669352ecf857ae6fd982873482c34
1282b7f34b examples: drop a couple allocations (Andrew Poelstra)
45e0241267 doc: fix "lazy line continuations" in markdown (Andrew Poelstra)
Pull request description:
Rust nightly as of 2024-05-27 has a new lint which detects list items which are continued by a non-indented line. Markdown treats these as single list items, which they sometimes are, but sometimes we intended them to be on a separate line.
Also changes the docs for `UntweakedKeypair::tap_tweak` because the existing ones were overly technical and out-of-date.
Also fixes a minor "no need to create a vec then dereference it" lint in the examples.
ACKs for top commit:
storopoli:
ACK 1282b7f34b
tcharding:
ACK 1282b7f34b
Tree-SHA512: 989c83b6bbbfbf8ddd2ade63b5f590f76810f4ec56fab432b580a40e3cc1468a0ea657dcc7ad0d4a31d49eb1390219616fec5a76b5680d5fddc3a06d81db30d7
b5ef7db3c0 api: Run just check-api (Tobin C. Harding)
1b40550ce8 Add an AddressData type (Tobin C. Harding)
Pull request description:
In the 0.32.0 release we removed the `address::Payload` struct because it was deemed an implementation detail. As a byproduct of doing so we made it impossible for users to match on an enum and get the address payload (or data).
- Add a public `AddressData` enum that holds an address' encoded data.
- Add a conversion function to `Address` that returns the data enum.
This patch is additive and is expected to be backported and release as a `0.32` point release.
ACKs for top commit:
apoelstra:
ACK b5ef7db3c0 I still feel a little partial to calling the struct "DecodedAddress" and the method "decode"...but this is good, and I do not want to bikeshed
Tree-SHA512: d97836bb2d7fc0f6e9fbba2afb30eeefefc88e7105d4765a146dd444c8397dd4d1ef4fd3e3eb925589294d46bfc8a66d33797a05dbc2131923534364424c135c
Rust nightly as of 2024-05-27 has a new lint which detects list items
which are continued by a non-indented line. Markdown treats these as
single list items, which they sometimes are, but sometimes we intended
them to be on a separate line.
Also changes the docs for `UntweakedKeypair::tap_tweak` because the
existing ones were overly technical and out-of-date.
In the 0.32.0 release we removed the `address::Payload` struct because
it was deemed an implementation detail. As a byproduct of doing so we
made it impossible for users to match on an enum and get the address
payload (or data).
- Add a public `AddressData` enum that holds an address' encoded data.
- Add a conversion function to `Address` that returns the data enum.
This patch is additive and is expected to be backported and release as a
`0.32` point release.
3615410d21 api: Run just check-api (Tobin C. Harding)
a3d2d1a184 Make Address:p2sh_from_hash public (Tobin C. Harding)
Pull request description:
We previously made this function Private and added a comment that doing so was somehow better to remove the footgun of hashing the wrong length script. However in hindsight this was a bad idea and users want the functionality.
Make the `Address:p2sh_from_hash` public and document it as we do for `Address::p2sh`.
This is an additive change and is expected to be backported to `v0.32`, as part of the fix to #2784. Please note it introduces the footgun that is described in the function rustdoc. This will be improved as a separate patch and added to the current release.
ACKs for top commit:
apoelstra:
ACK 3615410d21
Tree-SHA512: 535bb7894eeef8ecb5afb7bf6e5c483cd42c6a4282d1c116e5bf86cd1364a8327bbec1efb8634a578f07ad2832c1e5daf7fe7e844574b88b1ad355a443627bef
11bb1ff6ff Standardize function doc Safety, Returns and Parameters (jamil.lambert)
df83016c98 Standardize function doc Errors (jamil.lambert)
d219ceb68e Standardize function doc Examples (jamil.lambert)
233a9133d8 Standardize function doc Panics (jamil.lambert)
Pull request description:
The subheadings in the rustdocs have been standardized according to [./CONTRIBUTING.md](https://github.com/rust-bitcoin/rust-bitcoin/blob/master/CONTRIBUTING.md):
```rust
impl FooBar {
/// Constructs a `FooBar` from a [`Baz`].
///
/// # Errors
///
/// Returns an error if `Baz` is not ...
///
/// # Panics
///
/// If the `Baz`, converted to a `usize`, is out of bounds.
pub fn from_baz(baz: Baz) -> Result<Self, Error> {
...
}
}
```
ACKs for top commit:
apoelstra:
ACK 11bb1ff6ff
tcharding:
ACK 11bb1ff6ff
Tree-SHA512: 163af3cd1cfb47cea3e55eddeaeb6843ff7ec89c57354e3247d6bae85e756b183e8045c2555cfcf87e8c23c1388ff9d7592cfb6a951a37a9ec41d27263e5a2e4
cf3e1eb198 api: Run just check-api (Tobin C. Harding)
98bf213c52 bitcoin: Remove error module (Tobin C. Harding)
a5b93cb159 Flesh out hex unit parsing API (Tobin C. Harding)
Pull request description:
Add to `units::parse` the complete suit of hex unit parsing functions:
- remove prefix
- assert without prefix
- parse with or without prefix
- parse with prefix
- parse without prefix
- parse prefix unchecked
Refactor `bitcoin` to use the exact function we need, removing code duplication.
This is a breaking change to `units`, it does however keep the current re-exports from the public, now empty, `bitcoin::error` module.
ACKs for top commit:
apoelstra:
ACK cf3e1eb198
Tree-SHA512: 1778108d4364e290e8956cfea6f23fcdd82c835844d034a00b4cf5cab5552e3efbe853dfbf8a3e0a4bd53a8e3da9d6f7c7408d332d18cd7090aec16fc1f02fe7
We previously made this function Private and added a comment that doing
so was somehow better to remove the footgun of hashing the wrong length
script. However in hindsight this was a bad idea and users want the
functionality.
Make the `Address:p2sh_from_hash` public and document it as we do for
`Address::p2sh`.
726ff25c46 Hard code genesis script bytes instead of hex (Tobin C. Harding)
6e5592db77 Use test_hex_unwrap in bench code (Tobin C. Harding)
Pull request description:
Currently we have a dependency on `hex_lit` and it is used in exactly one place outside of test code, if we instead use a hardcoded array instead we can move the `hex_lit` dependency to `dev-dependencies`.
Hard code the genesis block script bytes as an array of hex digits, link to the blockstream explorer for those interested and comment the bytes liberally since it took me a while to work out what they were.
Move the `hex_lit` dependency and update the lock files.
ACKs for top commit:
apoelstra:
ACK 726ff25c46
Tree-SHA512: 96110332fc24dd5b251150b32737fa198113244c3b51b35453c8c1fcc8386c5a2f68dddb30d78cf2f9e1762550099fdb4109dc550f4c144625795ce60b86e574
7f29313d36 Update API (Tobin C. Harding)
4f29adf163 Enable getting the witness program from an address (Tobin C. Harding)
Pull request description:
We have getters for the pubkey hash and script hash but we forgot one for the witness program - add it.
Done as part of fixing #2784, this is an additive change and is expected to be backported to `v0.32`
ACKs for top commit:
apoelstra:
ACK 7f29313d36
Tree-SHA512: 365aba572eaacb789f5424c233be067500ceff18dc27e28fc3be123c49d27333e95bbf4527469a9caf8cc2fe54f8e13a0fc83bc3fc7c3356aea876c9dd2fa5f0
9bb75703a1 Header change from arguments to parameters (jamil.lambert)
Pull request description:
In a few cases a function header documents the parameters of the following function under the heading "Arguments", this has been changed to "Parameters".
Since the description is at the level of the function definition and not where it is being called parameters seems the more accurate term.
ACKs for top commit:
apoelstra:
ACK 9bb75703a1
tcharding:
ACK 9bb75703a1
Tree-SHA512: aa24af3fd6e086c09f5e2605fa58289969fc7188f63d7f53c0e325315644f9704d51d4cf526ebfc51b2cf9216155fc3d48cc6bca759dc14bae15e4770de5116e
The `error` module is empty except for public re-exports. We are still
in the "break everything and get the API right" stage so this module
adds no value - remove it.
Add to `units::parse` the complete suit of hex unit parsing functions:
- remove prefix
- assert without prefix
- parse with or without prefix
- parse with prefix
- parse without prefix
- parse prefix unchecked
Refactor `bitcoin` to use the exact function we need, removing code
duplication.
This is a breaking change to `units`, it does however keep the current
re-exports from the public, now empty, `bitcoin::error` module.
30a482504b bump nightly-version (Andrew Poelstra)
5ad7c245e3 cargo: whitelist all cfgs used in this repo (Andrew Poelstra)
814786b0a6 crypto: enable and fix accidentally disabled unit test (Andrew Poelstra)
Pull request description:
https://github.com/rust-lang/rust/issues/124800 has been fixed and we can update our nightly version by whitelisting all cfgs that are used.
There was one place where we had an old `cfg(feature = "no-std")` despite having removed the feature. By removing that cfg check we re-enabled a previously disabled test.
ACKs for top commit:
tcharding:
ACK 30a482504b
Tree-SHA512: d25bed819091db74b9d47cb2c23caa3ceb0d7be323b37831326e2ec1608cb1577d41aad2e1cdf59d66df69397537bc3e17a3c2872935d5a4f46f4dc55b5e613c
Currently we have a dependency on `hex_lit` and it is used in exactly
one place outside of test code, if we instead use a hardcoded array
instead we can move the `hex_lit` dependency to `dev-dependencies`.
Hard code the genesis block script bytes as an array of hex digits, link
to the blockstream explorer for those interested and comment the bytes
liberally since it took me a while to work out what they were.
Move the `hex_lit` dependency and update the lock files.
We would like to move the dependency on `hex_lit` to be a
dev-dependency but currently are using it in bench code. The bench
code is enabled if any downstream crate tries to build with
`--cfg=bench` and during such a build our dev-dependencies are not
available.
We also have the `test_hex_unwrap` macro in the `hex` crate and since
the bench code is more or less test code (and the macro call is not
being benchmarked) we can use that macro instead.
c8caee2b5e Document CompactTarget order/equality (Tobin C. Harding)
Pull request description:
Add documentation to the `CompactTarget` type explaining the nuance surrounding order/equality.
Close: #2110
ACKs for top commit:
apoelstra:
ACK c8caee2b5e
Tree-SHA512: c724b31ee620ff08d3c8b547250bc7067f875ef6cf4ce9efa082d5a9cfbd8b92620f86034e58caf573c479ce7aaa89bb7e9fa93dc356524663d3ecf583df3507
In a few cases a function header documents the parameters of the following function under the heading"Arguments", this has been changed to "Parameters"
Andrew Poelstra
Jose Storopoli
Tobin C. Harding
Divyansh Gupta
Shing Him Ng
Fmt Bot
Jamil Lambert, PhD
Antoni Spaanderman
Liu-Cheng Xu
Velnbur