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
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.
Currently the feature enabling is different for "std" and "no-std",
which is again different to the order in the dependencies section. These
two things make reading the manifest harder than it needs to be.
Put the dependencies in alphabetic order in the dependencies section as
well as when enabling them.
Refactor only, no logic changes.
b1870656c9 Combine the two Error impl blocks together (Tobin C. Harding)
82ea4ff31d Move error code to submodule (Tobin C. Harding)
67511ed03f Move no_std above comment (Tobin C. Harding)
f83d68f246 Add vertical whitespace (Tobin C. Harding)
Pull request description:
Do various refactorings to the `io` crate. These are all trivial to review except the bigish commit: `d3d3e50f Move error code to submodule` which is code move only.
ACKs for top commit:
Kixunil:
ACK b1870656c9
apoelstra:
ACK b1870656c9
Tree-SHA512: 391347614121ea8ebe55af705e9688a9a668222cb8f1eb0a3a5275970e67ffb9dd6dfbd436080ab72592f0c86f2b8b36e0ff0322ca25dcd926d0ce80fb8b942f
Move error code to a private `error` submodule and re-export it at the
crate root.
(Puts private `sealed` module at the bottom of the file i.e., this patch
is strictly a code move but we re-order the code while moving it.)
6a9713a0cc Add author for the format bot (Tobin C. Harding)
Pull request description:
The format bot defaults to using the person who triggered the workflow. We can use the `author` option to configure a dummy bot author.
ACKs for top commit:
Kixunil:
ACK 6a9713a0cc
apoelstra:
ACK 6a9713a0cc
Tree-SHA512: 8b2c419767d217ddb499032497f46588f939d92a01694b611ad44d872541871da28cfe3e9edd0c917bfdea02e86f8ed3a4fdfef60b59d0be5b7c0c2814a5db7b
c27f101840 Introduce usage of TBD instead of NEXT-RELEASE (Tobin C. Harding)
Pull request description:
There is an ecosystem-wide solution to our custom usage of `NEXT-RELEASE` in deprecation attributes - use
`#[decprecated(since = "TBD", note = "use bar instead")]`
Lets use it.
ACKs for top commit:
apoelstra:
ACK c27f101840
Kixunil:
ACK c27f101840
Tree-SHA512: 00cbb09f41249402f90e8b2da8c930adca9eec2e4e2684fb26eb844f738245a113788fde392b0b3bf4b5f278057e111ece7a805d0b19e93a4997873ba8051d2f
There is an ecosystem-wide solution to our custom usage of
`NEXT-RELEASE` in deprecation attributes - use
`#[decprecated(since = "TBD", note = "use bar instead")]`
Lets use it.
f764a607ac Use conventional import path for io crate (Tobin C. Harding)
5c0759a390 Inline io module in io crate root (Tobin C. Harding)
80fe9b99b2 Move public macros to a separate module (Tobin C. Harding)
Pull request description:
Its not immediately obvious why we nest the whole `io` code in an `io` submodule within `lib.rs`. As far as I can tell we can inline it and re-export from `rust-bitcoin` same as we do for our other dependencies.
This change would effect other users of the crate but since the `io` crate is unreleased this effects no-one except us.
After doing this it might be because `crate::io::Foo` looks good when near `std::io::Foo`?
ACKs for top commit:
apoelstra:
ACK f764a607ac
Kixunil:
ACK f764a607ac
Tree-SHA512: 38888b0c23d5f2cd874f77dd332fe4fa4b9acb90e3a2dac19e62ed3d98151acd7480c719aa85434e1a3de987af2c4f565528a914a14d5fd3f0f0e410cbdf5d40
We have a convention in `rust-bitcoin` to use external crates directly
when importing them not via `crate::foo`.
Update all the import paths for `io` to use this form.
fcc4c40a1c Rename from_vb_const (yancy)
Pull request description:
The new function is more clear because the purpose of the function is to return a value that doesn't need to be unwrapped. The current MSRV does not allow unwrap() in const context.
ACKs for top commit:
apoelstra:
ACK fcc4c40a1c
Kixunil:
ACK fcc4c40a1c
Tree-SHA512: 62bbd61800e9f29768d884dbe3bca14fea9b51b8f413131e0f29bfc0f0d0e20631d30489e078cc948f2dba0c5d7d9d7c229b4bb7187faef23083a88337efb6a6
The new function is more clear because the purpose of the function is to
return a value that doesn't need to be unwrapped. The current MSRV does
not allow unwrap() in const context.
Its not immediately obvious why we nest the whole `io` code in an `io`
submodule within `lib.rs`. As far as I can tell we can inline it and
re-export from `rust-bitcoin` same as we do for our other dependencies.
This change would effect other users of the crate but since the `io`
crate is unreleased this effects no-one except us.
In preparation for inlining the `io` molule, move the public macros to a
private `macros` module.
Includes removal of the public re-export of `std` as `_std` - flaggin
this because I do not understand why it is here in the first place, we
can use `std::io::Write` in code that is feature gated on "std".
761de886be Remove imports of TryFrom and TryInto (Tobin C. Harding)
4d5415f835 Add rust-version to the workspace manifests (Tobin C. Harding)
a41e978855 Update to edition 2021 (Tobin C. Harding)
d9cc724187 Bump MSRV to Rust version 1.56.1 (Tobin C. Harding)
Pull request description:
Rust version 1.56.0 introduced edition 2021. Shortly afterwards, on October 21 2021 Rust version 1.56.1 was released.
Debian stable is currently shipping `rustc 1.63.0`. Our stated MSRV policy is: In Debian stable and at least 2 years old. Therefore our MSRV policy is met by Rust version 1.56.1 and we can strat to bump our MSRV org wide. Start by bumping the `rust-bitcoin` and `hashes` MSRV to Rust 1.56.1
Start by bumping the `rust-bitcoin` and `hashes` MSRV to Rust 1.56.1, includes:
- Update docs.
- Update CI and remove pinning.
- Update the build files and remove now stale cfg attributes rust_v_1_x for values less than the new MSRV.
- Use new `IntoIterator` for arrays so we no longer need to allocate a vector to iterate.
Links:
- https://blog.rust-lang.org/2021/11/01/Rust-1.56.1.html
- https://blog.rust-lang.org/2021/10/21/Rust-1.56.0.html
- https://packages.debian.org/stable/rust/rustc
ACKs for top commit:
Kixunil:
ACK 761de886be
apoelstra:
ACK 761de886be
Tree-SHA512: 3a81c8bfa37d8cec0ec794f516f014da67ae8e437decf149c9681aa547885acac0ee07ea2c0f42e4f6bfd6f7ed1695fcf4747f53cc50e5f4e70ce3fe7bcba4e9
fa104aefa5 bitcoin: Add signing examples (Tobin C. Harding)
Pull request description:
Add two signing examples to showcase signing a simple one input two output transaction using both segwit v0 outputs and taproot outputs.
This patch is the result of the recent rust-bitcoin TABConf workshop, with bug fix by Sanket, updated to use APIs from tip of master branch.
This code, depending on v0.30.0 is what was added to the cookbook.
ACKs for top commit:
realeinherjar:
ACK fa104aefa5
apoelstra:
ACK fa104aefa5
Tree-SHA512: ce0d5b8291c94387c68b5e1cf740d3267fc00c997af5b96f5be525f348140d9a9af17ab66d556990f09bf081a5a812374cb633ea276100c7c21f218b85eae3fd
f41ebc2149 Add test for input weight predictions (conduition)
4514a80a23 Fix the InputWeightPrediction constants for DER signatures (conduition)
b5ce219c62 add weight method to InputWeightPrediction (conduition)
Pull request description:
The `P2WPKH_MAX` constant assumed DER signatures in the witness have a max length of 73. In practice, their maximum length is 72, because [BIP62](https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki) forbids nodes from relaying transactions which contain non-canonical ECDSA signatures (i.e. TX sigs must have an $s$ value of less than $\frac{n}{2}$).
This means $s$ is never encoded with a leading zero byte, and the signature as a whole never exceeds 72 bytes in total encoded length. The `ground_p2wpkh` function was already correct; only the constant needed to be corrected.
Technically 73 bytes *is* the upper limit for signatures, as nothing forbids miners from including such non-standard transaction signatures in blocks, but for the purposes of fee estimation and input weight prediction, 72 is the number which 99.9% of implementations should use as their ceiling. We already use it as the ceiling for the `ground_p2wpkh` function - `ground_p2wpkh(0)` returns a prediction which uses a witness signature of length 72.
Reference:
- https://bitcoin.stackexchange.com/questions/77191/what-is-the-maximum-size-of-a-der-encoded-ecdsa-signature
- https://bitcoin.stackexchange.com/questions/106435/are-high-s-ecdsa-signatures-forbidden-in-segwit-witnesses
- https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki
To enable testing, I added a `weight()` method to `InputWeightPrediction` and made it public but i'm not sure whether it has a use-case. Let me know if I should make it private instead.
ACKs for top commit:
tcharding:
ACK f41ebc2149
apoelstra:
ACK f41ebc2149
Tree-SHA512: 10e837bad9881c0efebb0598eaefd4ab039f2a6ececead75a68e253d84f5e85cb30496a6069eee8dfe9714773f3aa23cfe373f5d88d1c5609e1b1be1ff142e37
Rust version 1.56.0 introduced edition 2021. Shortly afterwards, on
October 21 2021 Rust version 1.56.1 was released.
Debian stable is currently shipping `rustc 1.63.0`.
Our stated MSRV policy is: In Debian stable and at least 2 years old.
Therefore our MSRV policy is met by Rust version 1.56.1 and we can strat
to bump our MSRV org wide.
Start by bumping the `rust-bitcoin` and `hashes` MSRV to Rust 1.56.1,
includes:
- Update docs.
- Update CI and remove pinning.
- Update the build files and remove now stale cfg attributes rust_v_1_x
for values less than the new MSRV.
- Use new `IntoIterator` for arrays so we no longer need to allocate a
vector to iterate.
Links:
- https://blog.rust-lang.org/2021/11/01/Rust-1.56.1.html
- https://blog.rust-lang.org/2021/10/21/Rust-1.56.0.html
- https://packages.debian.org/stable/rust/rustc
75c490c60f hashes: Remove default features from schemars dep (Tobin C. Harding)
1105876423 Remove whitespace character from string (Tobin C. Harding)
a6d7d542ab bitcoin:: Remove dev dependency serde_derive (Tobin C. Harding)
Pull request description:
Done while investigating removal of `serde_derive` dependency.
- Patch 1: Do trivial dev-dep removal
- Patch 2: Manually implement `JsonSchema` and remove default dependencies from "schemars" dependency (transitively depends on `serde_derive`)
ACKs for top commit:
apoelstra:
ACK 75c490c60f
Tree-SHA512: aab5bd622a76fc24259933af2f20f863d20c8ccf6e69e68246c374266c540e483ced8a769532582a184b922996857db7320a6b08ae9b5b95503eac752ef9d301
321d3923b8 Add from_vb_const function (yancy)
Pull request description:
This function is can be used to construct a Weight type from_vb in const context. Note I don't think it's possible to test the panic case since it's a compile time error work around currently to panic.
ACKs for top commit:
tcharding:
ACK 321d3923b8
apoelstra:
ACK 321d3923b8
Tree-SHA512: dc11409f0e3079400da261a8c9f580ef0527b77643ce1a5dda65c0975db19c2f2da46ac693e6a2bf49e0105b8b096e1ee51f09f5d1c78d634e4e274d7467ee05
The P2WPKH_MAX constant assumed DER signatures in the witness have
a max length of 73. However, their maximum length in practice is 72,
because BIP62 forbids nodes from relaying transactions whose ECDSA
signatures are not canonical (i.e. all sigs must have an s value of
less than n/2). This means s is never encoded with a leading zero
byte, and the signature as a whole never exceeds 72 bytes in total
encoded length. The ground_p2wpkh function was already correct;
only the constant needed to be corrected.
Tobin C. Harding
Andrew Poelstra
apoelstra
yancy
conduition
Vojtěch Toman