43b1ed1b86 Fully encapsulate bitcoinconsensus (Tobin C. Harding)
Pull request description:
The `bitcoinconsensus` crate is not fully under our control because it exposes code from Core, so we cannot guarantee its stability across versions. To make our semver compliance easier we can fully encapsulate the `bitcoinconsensus` crate so it does not appear in our public API.
### Please note that with this applied:
- The `bitcoinconsenus` crate is no longer exported at the crate root
- No `bitcoinconsensus` types appear in our public API
ACKs for top commit:
Kixunil:
ACK 43b1ed1b86
apoelstra:
ACK 43b1ed1b86
Tree-SHA512: 9fc4f01a35396562e980a647784b22667cbd289e45b5c122610d23a1f8bcf0fe8b9c27e33745f14ee010050d4c2d2669b679fb39c7a108e4e86d2c14fd60571a
The `bitcoinconsensus` crate is not fully under our control because it
exposes code from Core, so we cannot guarantee its stability across
versions. To make our semver compliance easier we can fully encapsulate
the `bitcoinconsensus` crate so it does not appear in our public API.
However, it is useful to have the crate itself exported, here we add an
"unstable" feature and only publicly export the `bitcoinconsensus` crate
if the "unstable" feature is enabled.
f06d12455f bitcoin: Remove the custom sink (Tobin C. Harding)
b503aa1544 Run the formatter (Tobin C. Harding)
3ca55fb163 Remove qualifying path from Read and Write (Tobin C. Harding)
ebeb21fa7a Import fmt::Write using underscore (Tobin C. Harding)
e2dbcb1d28 Use W for writer generic type (Tobin C. Harding)
8704d9f0ae docs: Fix grammar (Tobin C. Harding)
Pull request description:
A few cleanups to how we use the `io` crate, this is reasonably trivial but commit `a6c7e696 Remove qualifying path from Read and Write` is big, I have however gone to some effort to make sure it is easy to flick through the diff.
Done in preparation for another go at the `BufRead` stuff.
ACKs for top commit:
apoelstra:
ACK f06d12455f
Kixunil:
ACK f06d12455f
Tree-SHA512: 751c489c67901c7563f1cc91f7761a4e3c276ae1981010338134e8c13200720ba69fcc74948c1dc1e6e65390197da0da27b2b69b86034029748321b404142cba
daa47b2061 Allow `SignedAmount` parse values equal to i64::MIN (Jiri Jakes)
Pull request description:
Fixes#2267.
Previously, parsing such textual value returned 'too big' error. This change fixes it and adds relevant tests. Without this patch, the newly added tests fail with `TooBig`.
ACKs for top commit:
Kixunil:
ACK daa47b2061
apoelstra:
ACK daa47b2061
Tree-SHA512: b6e4ee8ed3fa15ecb988b3a6f32f16a17ba9c6de0b99c8a198254d85d263cfffa0498487a3f47b7ddc2c976cade128ac6a7952baecc8721d5d45e057c0eb87b4
There is no advantage in having `io::Read` as opposed to `Read` and
importing the trait. It is surprising that we do so.
Remove `io::` path from `io::Read` and `io::Write`. Some docs keep the
path, leave them as is. Add import `use io::{Read, Write}`.
Refactor only, no logic changes.
When we use the `fmt::Write` trait it is just to call its methods, we
can therefore, without any change to the logic, use `as _` when
importing the trait. This prevents naming conflicts.
Done in preparation for importing the `io::Write` trait.
Generic types can be single letters, and a writer is conventionally, in
this codebase at least, called `W`.
Use `W` instead of `Write` with no loss of clarity.
ae07bdbdbc Remove ToOwned from prelude (Tobin C. Harding)
Pull request description:
We are not using the `ToOwned` trait, remove it.
Found by clippy.
ACKs for top commit:
apoelstra:
ACK ae07bdbdbc
Tree-SHA512: 8d7d271a279027bd449a2c23c5a5df2217d456c7ca2f286bbe57934f776bb10cb882ceabb52cdd41d5d622f864eefd2bcbb49b68dcfa27203039fd62edcd9d73
94a6caf204 hashes: Add Hash::from_byte_chunks to construct hashes from iterators (Steven Roose)
Pull request description:
Is there any interest in a method like this? I'm not necessarily a fan myself, was just in a situation where I was doing this a lot and it would save the me the "engine, from_engine dance" each time.
ACKs for top commit:
Kixunil:
ACK 94a6caf204
apoelstra:
ACK 94a6caf204
Tree-SHA512: 96e74366dbf64d50fb6642024d18eeeee5b78154fa152c845a1073c904599af69328a141f0a7e2bd1a7b43a091c886da57dfe6e87bab5a5e01b0f167019caae0
1ee989a3af Remove private fmt_internal function (Tobin C. Harding)
923ce7402d Remove Network from AddressInner (Tobin C. Harding)
3490433618 Return error from wpubkey_hash (Tobin C. Harding)
f7ab253ce4 Remove stale comment (Tobin C. Harding)
Pull request description:
An `AddressInner` struct (contains `Network` field) is created when parsing address strings however address strings do not map 1:1 to `Network` because signet and testnet use the same bech32 prefix "tb".
We can fix this by inlining the `Payload` variants into `AddressInner` and adding prefix enums for legacy addresses and an `Hrp` for bech32 addresses.
Fix: #1819
ACKs for top commit:
Kixunil:
ACK 1ee989a3af
apoelstra:
ACK 1ee989a3af
Tree-SHA512: 1c2749dc929a1e9ad9b9feb01bec5c96b5aec07c6d646d88652deca7abe485907403116e9e29a0ab7dc06223254c4b49a384043284ec0a68fd76f9ab551e9e8a
396e049a7a Use InputString instead of String (Tobin C. Harding)
acacf45edf Add ParseDenominationError (Tobin C. Harding)
69e56a64ed Add bitcoin-units crate (Tobin C. Harding)
4ecb1fe7da internals: Add docs to InputString (Tobin C. Harding)
fa8d3002cd internals: Fix docs typo (Tobin C. Harding)
Pull request description:
Create a new `bitcoin-units` crate as described [here](https://github.com/rust-bitcoin/rust-bitcoin/issues/550#issuecomment-1012103022).
Only the `amount` module is currently included.
I've resolved the `Encodale/Decodable` issue by keeping the `amount` module in `bitcoin`.
ACKs for top commit:
Kixunil:
ACK 396e049a7a
apoelstra:
ACK 396e049a7a
Tree-SHA512: caf5e9da0458435ab19d00d4506896257e898525a4472d435fdac1d1a37bb747befd56993b106673f938475e5777d952a13ba04a2d3cb710d7afe7f5faebb7b8
e1cc98986c Put `#[inline]` on trivial functions (Martin Habovstiak)
e531fa612b Move `TaprootMerkleBranch` and impl `IntoIterator` (Martin Habovstiak)
9d23c1d0a8 Implement std traits for `TaprootMerkleBranch` (Martin Habovstiak)
93b415589d Rename `inner` to `slice`/`vec` (Martin Habovstiak)
bb0f839c2f Lint with nightly (Martin Habovstiak)
Pull request description:
This contains several improvements to `TaprootMerkleBranch` that make the API more idiomatic.
ACKs for top commit:
tcharding:
ACK e1cc98986c
apoelstra:
ACK e1cc98986c
Tree-SHA512: b2bf52b027e7c1f8588c54e8b8d7a5fa54011dc521bd917995011d5fcc16c50a486eb89c0cdae2557a58adbe7708a4f2bc8f4c492e3d88c679f2abf85b1e7c83
1b23220d10 Fix: TxOut::minimal_non_dust and Script::dust_value (Jonathan Underwood)
Pull request description:
Fixes#2192
TxOut::minimal_non_dust has 3 problems.
1. There is an invisible dependency on Bitcoin Core's default minrelaytxfee value. It has been made explicit.
2. There is an off by one error. The dust limit comparison uses < and therefore `+ 1` was not needed. It has been fixed.
3. It was not returning 0 amount for OP_RETURN outputs.
Script::dust_value has 2 problems.
1. The dust amount depends on minrelaytxfee which is configurable in Bitcoin Core. This method was not configurable.
2. The division operation was done before multiplying the byte amount, which can cause small differences when using uncommon scripts and minrelaytxfee values.
ACKs for top commit:
Kixunil:
ACK 1b23220d10
apoelstra:
ACK 1b23220d10
Tree-SHA512: eafd5112fbf773d86e094e3a69c519dd32f5074f5c9c63a8d69b1c9796579a8f2c2d11ad0995d8252c25b7fed5cd7c968ab88a70588986981a0a63649d43e197
c7c553ebc0 Remove impossible InvalidParity error variant (Martin Habovstiak)
Pull request description:
Since we do `& 1`, only 0 and 1 are possible values, so the error return there can never happen. I made this explicit by manually setting the parity.
This is a rebase of Steven's change #2163 with a rewrite of `match` to not panic.
ACKs for top commit:
tcharding:
ACK c7c553ebc0
apoelstra:
ACK c7c553ebc0
Tree-SHA512: 5591fda686295f330b6da757a0052687eddec135ac947a2801343f1681bc4fd9f6cfb722ac1339ae6187a8784e7629b5f12cca32b33a81ffc8791b4407b29f85
3d17031725 Derive Debug for PrivateKey for no-std builds (Tobin C. Harding)
Pull request description:
Currently we derive `impl Debug for PrivateKey` for "std" builds and manually implement an obfuscated version for "no-std" builds. Since we enable the `hashes` feature of `rust-secp` this is unnecessary because secp takes care of obfuscating the secret for us.
ACKs for top commit:
apoelstra:
ACK 3d17031725
Kixunil:
ACK 3d17031725
Tree-SHA512: 0ce394c6517c51e8964290a980cddd20186d19bcc6cbb8c71aa09b7485d6a0df373960798418184971e1c6e5a6b8f725dd44ebfa7184e31b63faf105dea69725
801c72e056 Add deprecation comment to hash_types module (Tobin C. Harding)
61351c917f Move impl_asref_push_bytes to internal_macros (Tobin C. Harding)
2b4b66dee3 Move impl_hashencode to internal_macros (Tobin C. Harding)
2a0ac1258a Move the bip158 filter hash types (Tobin C. Harding)
3107f80aac Move transaction hash types (Tobin C. Harding)
61c02ff202 Move block hash types (Tobin C. Harding)
Pull request description:
Move hash types out of `hash_types` and into the modules where they are primarily used. Adds deprecated re-export so this is not a breaking change.
Is an alternate solution to #2072Resolves: #2072
ACKs for top commit:
apoelstra:
ACK 801c72e056
Kixunil:
ACK 801c72e056
Tree-SHA512: 4ccac63553de3f7d417213429c0f5c2b7ebc3c2d77a9feb6d4a7daa233565fc62617edf6426a421d251eadc0841235a719bd7fd3f980302c7a2bf3dacb8b4a61
Since we do `& 1`, only 0 and 1 are possible values, so the error return
there can never happen. I made this explicit by manually setting the
parity.
This is a rebase of Steven's change with a rewrite of `match` to not
panic.
Since the iterator created by `IntoIterator` should be called `IntoIter`
we move the whole `TaprootMerkleBranch` to its own module which contains
the type to avoid confusion. This has an additional benefit of reducing
the scope where the invariant could be broken. This already uncovered
that our internal code was abusing access to the private field (although
the code was correct).
To implement the iterator we simply delegate to `vec::IntoIter`,
including overriding the default method which are likely to be
implemented by `Vec` more optimally. We avoid exposing `vec::IntoIter`
directly since we may want to change the representation (e.g. to
`ArrayVec`).
The type is naturally a collection of hashes so make it behave that way
by implementing `Deref`, `AsRef`, `Borrow` and their mutable versions as
well as `IntoIterator` for its reference. `IntoIterator` for itself is
not yet implemented because it's a bit more complicated.
b058030384 Add deprecated bip32 types back in (Tobin C. Harding)
Pull request description:
In the 0.31.0 release we renamed the bip32 extended key types without leaving the originals in there marked as deprecated. This makes for a bad experience for devs, add them back in.
ACKs for top commit:
Kixunil:
ACK b058030384
apoelstra:
ACK b058030384
Tree-SHA512: 215c5be0953d45e0fcd31cd626b8e15d60c776ef1ccaab5e7047077b323223341f890487ba69363379383b5ac16469e65d6576706ffe6366d7dc96e6bb99b7f9
While `clippy` now allows `TBD` to be used in `since` parameter of
`deprecated` attribute it is only available in the newest, nightly,
version. Switch `clippy` version to nightly to enable the `TBD` value.
TxOut::minimal_non_dust has 3 problems.
1. There is an invisible dependency on Bitcoin Core's default minrelaytxfee value. It has been made explicit.
2. There is an off by one error. The dust limit comparison uses < and therefore `+ 1` was not needed. It has been fixed.
3. It was not returning 0 amount for OP_RETURN outputs.
Script::dust_value has 2 problems.
1. The dust amount depends on minrelaytxfee which is configurable in Bitcoin Core. This method was not configurable.
2. The division operation was done before multiplying the byte amount, which can cause small differences when using uncommon scripts and minrelaytxfee values.
In the 0.31.0 release we renamed the bip32 extended key types without
leaving the originals in there marked as deprecated. This makes for a
bad experience for devs, add them back in.
98ce46c009 Update docs on witness_mut (Tobin C. Harding)
Pull request description:
Recently during the rust-bitcoin workshop at TABConf devs were thrown off by the example on `witness_mut`. We have some work going on to add examples and a cookbook that all demonstrate usage of `witness_mut`.
Remove the docs on `witness_mut` and direct devs to the `examples/sign-tx-*` files.
ACKs for top commit:
apoelstra:
ACK 98ce46c009
Kixunil:
ACK 98ce46c009
Tree-SHA512: e662213db4cbdaa53f6927cc1b10c1b6276f538cc6ad0d4bfff6dfcbf042f287a14bf5bfc88eeba7a32646c3d6741c5e09d11bb76666572a12a2043db55a2f38
Recently during the rust-bitcoin workshop at TABConf devs were thrown
off by the example on `witness_mut`.
Attempt to improve the docs on `witness_mut`.
0ac9ad16ce Add `taproot::SerializedSignature` (Martin Habovstiak)
dffa51e735 Move taproot module to a subdirectory (Martin Habovstiak)
Pull request description:
Previously `taproot::Signature` could be only serialized into `Vec<u8>`
which forced allocation. This adds a `SerializedSignature` type which
acts like `Box<u8>` but is on stack.
Note: the code was copied from `secp256k1::ecdsa::serialized_signature`
with minimal changes.
ACKs for top commit:
apoelstra:
ACK 0ac9ad16ce
tcharding:
ACK 0ac9ad16ce
Tree-SHA512: e3d24f4ddd8074d477c25f5378c3f57dd266405380cc54596104effbad96e7abdb201e032b4b70cdbbaac595d9ae9c4043fca79d275ce62f9f6b221e6783956e
6f1cb42d84 Remove NEXT_RELEASE from release script comment (Tobin C. Harding)
Pull request description:
Recently we remove the usage of `NEXT_RELEASE` in favour of `TBD` but I missed the code comment.
ACKs for top commit:
apoelstra:
ACK 6f1cb42d84
Kixunil:
ACK 6f1cb42d84
Tree-SHA512: e2b96cbd053fa95a9dc4f432f141822cd4ce4bffcbf6a6a0f5ba163c66743a776bc1268be0ec62c28590e942fe75f906e99a22e6a46cb11180c59bd1be39901c
b72f1d70e5 bitcoin: Add DO_FEATURE_MATRIX (Tobin C. Harding)
48879e7ad9 Remove no-std feature (Tobin C. Harding)
7d71fb9fdb Re-order dependencies in manifest (Tobin C. Harding)
Pull request description:
Now we have the `io` crate we can remove the "no-std" feature.
Do a few cleanups to the `bitcoin` manifest, then remove the "no-std" feature - BOOM!
ACKs for top commit:
Kixunil:
ACK b72f1d70e5
apoelstra:
ACK b72f1d70e5
Tree-SHA512: fb14f70e5ef7882437fefe059eafb112684ccb0077551114b7ae66fa3576cb01fec781a6fbaa24f57d66537109a5815037790df87d8665f97614d6615f49c6cb
Previously `taproot::Signature` could be only serialized into `Vec<u8>`
which forced allocation. This adds a `SerializedSignature` type which
acts like `Box<u8>` but is on stack.
Note: the code was copied from `secp256k1::ecdsa::serialized_signature`
with minimal changes.
Add a feature matrix section to the `bitcoin` CI script as we do in
`hashes`. This means:
- test with no default features
- test with all individual features
- test with all combinations of two features
Note, with this applied all features and optional dependencies are
included in `FEATURES` (excluding `secp-lowmemory`).
The feature matrix test gets run for stable and MSRV toolchains.
Currently `bitcoin` cannot be built with no features enabled, it must
have either "no-std" or "std" enabled. This is an artifact from when
we depended on `core2` for "no-std", now that we have our own `io` crate
and we unconditionally depend on it we can remove the "no-std" feature.
We are emptying the `hash_types` module. `impl_asref_push_bytes!` is an
internal macro, as such it can live in the `internal_macros` module.
While we are at it import the macro and call it without any qualifying
path, this is typical for our usage of other internals/internal_macros
usage.
Andrew Poelstra
Tobin C. Harding
Jiri Jakes
Steven Roose
Fmt Bot
Martin Habovstiak
Jonathan Underwood