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"
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.
The formatter lines up comments if they are on consecutive lines even
if the second is supposed to be at the start of the collum and the
first is after code. Putting a line of whitespace between the two
lines stops this from happening.
Add whitespace to stop the formatter doing silly changes.
Whitespace only.
Clean up the test imports in the `p2p` module:
- Use `use super::*` as is conventional.
- Use `sha256d::Hash` as is conventional.
Refactor, no logic changes.
6d0d0fe51f Fix rustdoc header format (jamil.lambert)
Pull request description:
By convention rustdoc headers should not include a colon.
Removed colon from rustdoc headers.
ACKs for top commit:
apoelstra:
ACK 6d0d0fe51f
Tree-SHA512: 36d5822e80cc86139ce07a1c86681998e1d27549421b9b50589fe4a9a5c860ed47f145d5af9501957f199d0f77df069c2b295d43ed4a15e9b6d98c0edc12abd0
802af8e417 Removed //! spare line at end of headers (jamil.lambert)
Pull request description:
Some of the headers had a //! at the end but most didn't. They have all been removed in bitcoin/src/ to make the files consistent
ACKs for top commit:
apoelstra:
ACK 802af8e417
Tree-SHA512: a1eb0dda76af68cb96352f6b31231fa5391d49e11df924065e76871f82231ec0d5751190663f142240e5d757975937387243d1fdac3684d9bdbd7e2362dbd0a7
Currently we use the `Hash` trait in a bunch of places to call
`all_zeros`. We are attempting to improve the `hashes` API and this
usage is both unnecessary and also hindering that effort.
Use the concrete type (e.g. `BlockHash`) instead of calling through the
trait method.
Refactor only, no logic changes.
47dc4a3180 feat(pow): add difficulty adjustment calculation (Rob N)
Pull request description:
Hi, I hit a roadblock with the current `pow` API. As far as I can tell, the only workaround to calculate the next work required similar to `bitcoin/src/pow.cpp` is to use a general big integer library, convert the `Target` to bytes, do the math, and convert back to `Target` from bytes. I have also been working with [Floresta](780ea8d0b0) and their [solution](780ea8d0b0/crates/floresta-chain/src/pruned_utreexo/consensus.rs (L187)) was to fork off and exposed the `U256` struct publicly on their branch. I think these home brewed difficulty adjustment solutions will continually pop up, so I created a `from_next_work_required` method to return a `Target`. My work veers significantly from #2180, as I only provided a single method to do so, without further guidance on when exactly this retarget occurs.
I am happy to add tests once I get further direction from maintainers if this as a likelihood of being accepted or not. Thanks.
ACKs for top commit:
tcharding:
ACK 47dc4a3180
apoelstra:
ACK 47dc4a3180 used range-diff
Tree-SHA512: 6d627ce698361afed61c8f2a12a1a96371a7a93118e08a91dae250de4f23d65c615d2654d37d2699c88b7c22f6e4bc2a1195f963c15512d7c0d041498f02dc41
46e0ce59a3 bitcoin: Set version number (Tobin C. Harding)
Pull request description:
During the recent release cycle we left `bitcoin` on the last rc version.
Set the version number to `v0.33.0-unreleased` to make it obvious what it is.
Close: #2724
ACKs for top commit:
apoelstra:
ACK 46e0ce59a3
sanket1729:
ACK 46e0ce59a3
Tree-SHA512: 8a1fa062cf0240c5a7e1c05f0378097464423c29827f14068d01ac562d1be571bec2d267a5a12d0828c5a2809ef743f03ef29ccea9a31a5c9e3eefce66b3d30d
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.
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` for functions
that return arrays
- Add `as_bytes` to return a slice
- Add `to_bytes` to return a vector
- Re-name the other macro to match what it now does
d353be4546 bip32: derive_xpriv should not return a Result (Jose Storopoli)
Pull request description:
We discussed in #2752 that `derive_priv` never fails.
This PR addresses that issue.
ACKs for top commit:
apoelstra:
ACK d353be4546
tcharding:
ACK d353be4546
sanket1729:
ACK d353be4546
Tree-SHA512: 3a3d09027c6079581636b51a506fac2b325e592d182167c0a3d8676b41fb2bef59a85a404ad12b2e14d73e58fd4b6d8f5923a3dc76a4b9724033097fc08b36ac
7685461e62 Document the sha256t_hash_newtype direction (Tobin C. Harding)
30e91cc766 Default to forward for tagged hashes (Tobin C. Harding)
5ecc69cd28 Add forward/backward unit test (Tobin C. Harding)
9aee65d1ba Refactor tagged hash tests (Tobin C. Harding)
216422dffc Remove schemars impl for test type (Tobin C. Harding)
Pull request description:
First three patches are preparation, improvements to the units tests in `sha256t`.
From the final patch:
Displaying backward is an anomaly of Bitcoin Core's early days and the
double SHA256 hash type. We should not let this unfortunate beast leak
out into other places.
Default to displaying forward when creating a new tagged hash and remove
all the explicit attributes from `bitcoin` that just clutter the code.
This is an API break and may quietly break some users downstream - eventually we should stop doing that sort of thing.
ACKs for top commit:
apoelstra:
ACK 7685461e62
Tree-SHA512: cb8a41b207aa68ecf63cb7af7f39f7d7c8a3a27f38595867949b288a81a20bff0c17aa4c17bb099e2ecf85194d83bad23c9c9792f511b6c4cd625ff27c1affaa
Currently we require indexing trait bounds as well as `Borrow` on the
`Hash` trait. We also already implement `AsRef`.
It was observed that `Borrow<[u8]>` does not best describe what we want
from the `Hash` trait implementor but rather `AsRef<[u8]>` does.
Remove all the inexing trait bounds. Remove the `borrow::Borrow<[u8]>`
trait bound. Add a `convert::AsRef<[u8]>` trait bound.
This leaves the `Borrow<[u8]>` implementation for hashes created with
`hash_newtype`, I'm not sure if this should be removed or not.
During the recent release cycle we left `bitcoin` on the last rc
version.
Set the version number to `v0.33.0-unreleased` to make it obvious what
it is.
Close: #2724
12411fc917 Fix typo in deprecated BIP-32 type (matthiasdebernardini)
Pull request description:
In #2258 we attempted to add back in deprecated BIP-32 types - but we spelled the identifier incorrectly. The patch was then backported to the `0.31.x` branch in December but was only just noticed now.
Fix typo in deprecated type from `Extendend` -> `Extended`.
ACKs for top commit:
tcharding:
ACK 12411fc917
storopoli:
ACK 12411fc917
apoelstra:
ACK 12411fc917
Tree-SHA512: f70e8fe741740f62b29932d8ee84cbe7803cb71dfb0491d251c3a982ede07ea7a32b5ecdf569d6012ee05509e8182a439b022c606a2f01742f4908089edc85a9
In PR #2258, deprecated BIP-32 types were re-added but contained a typo in the identifier: "Extendend" instead of "Extended". This commit fixes that typo.
The incorrect patch was backported to the 0.31.x branch in December but only noticed recently.
26b9782d8b CI: Re-write run_task.sh (Tobin C. Harding)
Pull request description:
Recently we re-wrote CI to increase VM level parallelism, in hindsite this has proved to be not that great because:
- It resulted in approx 180 jobs
- We are on free tier so only get 20 jobs (VMs) at a time so its slow to run
- The UI is annoying to dig through the long job list to find failures
Have another go at organising the jobs with the main aim of shortening total run time and making it easier to quickly see fails.
Re-write the `run_task.sh` script, notable moving manifest handling to the workflow. Also don't bother testing with beta toolchain.
### Note on review
The diff is hard to read for `rust.yml`, I tried splitting out a bunch of separate patches but it resulted in the same thing (because there are so many identical lines in the yaml file). I suggest just looking at the yaml file and not the diff.
ACKs for top commit:
apoelstra:
ACK 26b9782d8b
sanket1729:
ACK 26b9782d8b.
Tree-SHA512: 1b0a0bab5cf729c5890f7150953499b42aebd3b1c31a1b0d3dfa5b5e78fda11e17a62a2df6b610ab4a950d5709f3af6fff1ae64d9e67379338903497ab77ae0e
b355740da4 chore: format and standardize all markdowns files (Jose Storopoli)
Pull request description:
according to the github flavor
(https://github.github.com/gfm/)
ACKs for top commit:
apoelstra:
ACK b355740da4 thanks for going through all this!
tcharding:
ACK b355740da4
Tree-SHA512: a9d5671ddc6f922b42189cae11b2a2a877663c909f73f1e8c407a4de7ac93b291e435373b79be2c708f1ecb717b2ede147c0f7730d582a1cb5bee937603005f0
1c836acf30 bitcoin: Stop slicing hashes (Tobin C. Harding)
Pull request description:
As part of the ongoing effort to improve `hashes`; stop using slicing of hash types and use `as_byte_array()` to get an array reference instead. This gives us more flexability to modify the `hashes` module.
ACKs for top commit:
apoelstra:
ACK 1c836acf30
storopoli:
ACK 1c836acf30
clarkmoody:
ACK 1c836acf30
Tree-SHA512: ca4becfc9bae13127aae09bd1eb5e03efc670fb4e56c2aba1cf0e3cf5f5a762dd8c5cff8ff7c15167902b7440f70202e0884f1e4d3bb651476019c78d2be3408
Recently we re-wrote CI to increase VM level parallelism, in hindsite
this has proved to be not that great because:
- It resulted in approx 180 jobs
- We are on free tier so only get 20 jobs (VMs) at a time so its slow to run
- The UI is annoying to dig through the long job list to find failures
Have another go at organising the jobs with the main aim of shortening
total run time and making it easier to quickly see fails.
Re-write the `run_task.sh` script, notable moving manifest handling
to the workflow. Also don't bother testing with beta toolchain.
WASM Note
Removes the `cdylib` and `rlib` from the manifest patching during wasm
build - I do not know the following:
- Why this breaks on this PR but not on other PRs
- Why I can't get wasm test to run locally on master but PRs are passing
- What the `cdylib` and `rlib` were meant to be doing
This is the docs from: https://doc.rust-lang.org/reference/linkage.html
* --crate-type=cdylib, #![crate_type = "cdylib"] - A dynamic system
library will be produced. This is used when compiling a dynamic library
to be loaded from another language. This output type will create *.so
files on Linux, *.dylib files on macOS, and *.dll files on Windows.
* --crate-type=rlib, #![crate_type = "rlib"] - A "Rust library" file
will be produced. This is used as an intermediate artifact and can be
thought of as a "static Rust library". These rlib files, unlike
staticlib files, are interpreted by the compiler in future linkage. This
essentially means that rustc will look for metadata in rlib files like
it looks for metadata in dynamic libraries. This form of output is used
to produce statically linked executables as well as staticlib outputs.
As part of the ongoing effort to improve `hashes`; stop using slicing of
hash types and use `as_byte_array()` to get an array reference instead.
This gives us more flexability to modify the `hashes` module.
If one writes signing data using one of the two
`*_encode_signing_data_to` functions then creating the message to sign
is slightly nuanced and different for each of the functions. For Taproot
one must use a specific tagged hash and for ECDSA one must use a sha256d
hash.
Add documentation that explains the hashing requirements for each
function.
Displaying backward is an anomaly of Bitcoin Core's early days and the
double SHA256 hash type. We should not let this unfortunate beast leak
out into other places.
Default to displaying forward when creating a new tagged hash and remove
all the explicit attributes from `bitcoin` that just clutter the code.
f96bbebdcc Set release version in deprecated attribute (Tobin C. Harding)
Pull request description:
In preparation for release replace "TBD" with the next release version - `v0.32.0`.
ACKs for top commit:
apoelstra:
ACK f96bbebdcc
storopoli:
ACK f96bbebdcc
Tree-SHA512: 7478808322357d853fab2bf25a7d42a972d5ee05ed6f206bfb73748efe1154fb392dc76c3d0e1a50314bcfdac3a55a415f3c6d40dfaaab802ae1c69dd1ad9e76
In the segwit signing example we are using the incorrect value when
creating the signature - we should be using the utxo amount (input
amount) not the spend amount (output spend amount).
Close: #2680
30a09670e8 Add docs for custom signets (Tobin C. Harding)
Pull request description:
We have started using `AsRef<Params>` in a few places as a function parameter. If a user of the library wishes to use these functions they need to create a type that can implement this trait. Because we use `non_exhaustive` on the `Params` struct it is not possible to just construct a `Params` type. This may be surprising for some folk.
Add module level docs to the `consensus::params` module with an example of how to create a type that can be used to describe a custom signet network. Use fields inspired by Mutiny Wallet's described usage.
Close: #2690
ACKs for top commit:
sanket1729:
ACK 30a09670e8.
apoelstra:
ACK 30a09670e8 this is great; would like to see more `const` but for example code no big deal
Tree-SHA512: 50881763aea99641e24871b0eae60650174c48f620742944e7d5617fcf1edff73a20b2a8f043433f6f114ff5f3f4691703fc37b28880c305bb052c2d75d1eeeb
We have started using `AsRef<Params>` in a few places as a function
parameter. If a user of the library wishes to use these functions they
need to create a type that can implement this trait. Because we use
`non_exhaustive` on the `Params` struct it is not possible to just
construct a `Params` type. This may be surprising for some folk.
Add module level docs to the `consensus::params` module with an example
of how to create a type that can be used to describe a custom signet
network. Use fields inspired by Mutiny Wallet's described usage.
Close: #2690
I'm not sure why I haven't see this before during the whole test cycle
but while running `cargo kani --only-codegen` we get a bunch of warnings
of form:
warning: use of deprecated field `consensus::params::Params::pow_limit`
We deprecated the `pow_limit` field but still set it (obviously) in
const structs - just shoosh the warning.
830c1e9cfe Allow m prefix in derivation paths (Tobin C. Harding)
Pull request description:
Recently in #2451 we disallowed bip32 derivation paths with the leading 'm' variable.
There is some confusion as to what exactly the bip specifies however Bitcoin Core RPC call `getaddressinfo` returns a derivation path with a leading "m/". This means we need to be able to parse it irrespective of what the bip says.
Be more liberal in what we accept as a derivation path, including both with and without the leading 'm/'.
Leave the full investigation of the bip to a later date.
Change back some of the test strings as makes sense and include test strings to showcase the full current behaviour.
This PR replaces #2674.
ACKs for top commit:
apoelstra:
ACK 830c1e9cfe
sanket1729:
ACK 830c1e9cfe
junderw:
ACK 830c1e9cfe
Tree-SHA512: 7a4fccd49cb8cd91a6c8db51d758ae116d9d2e98fead7b87520ca302022b37ddbcf3f85453941c5f336f8e934ad224beba99527dc29ce8368fbb1f25508c1615
Recently in #2451 we disallowed bip32 derivation paths with the leading
'm' variable.
There is some confusion as to what exactly the bip specifies however
Bitcoin Core RPC call `getaddressinfo` returns a derivation path with a
leading "m/". This means we need to be able to parse it irrespective of
what the bip says.
Be more liberal in what we accept as a derivation path, including both
with and without the leading 'm/'.
Leave the full investigation of the bip to a later date.
Change back some of the test strings as makes sense and include test
strings to showcase the full current behaviour.
The deprecation notice for `is_provably_unspendable` contains "is not
very useful" which is a bit presumptuous to tell to users, it may very
well be useful to them. Use the more helpful text that already exists in
rustdoc on the function.
Andrew Poelstra
Jamil Lambert, PhD
Tobin C. Harding
Antoni Spaanderman
Liu-Cheng Xu
Jose Storopoli
Fmt Bot
Velnbur
Divyansh Gupta
Rob N
matthiasdebernardini