After MSRV bump `try_from` usually refers to `TryFrom` method but
`CommandString` used inherent one making it confusing and non-idiomatic.
This implements `FromStr` and `TryFrom<{stringly type}>` for
`CommandString` and deprecates the inherent method in favor of those.
To keep the code using `&'static str` efficient it provides
`try_from_static` inherent method that only converts from
`&'static str`.
Closes#1135
Add a `LockTime` type to hold the nLockTime `u32` value. Use it in
`Transaction` for `lock_time` instead of a `u32`. Make it public so this
new type can be used by rust-miniscript and other downstream projects.
Add a `PackedLockTime` type that wraps a raw `u32` and derives `Ord`,
this type is for wrapping a consensus lock time value for nesting in
types that would like to derive `Ord`.
Integers within Script can have a maximum value of 2^31 (i.e., they are
signed) but we (miniscript) often uses unsigned ints, to facilitate
checking the unsigned type is the correct size to fit in a signed int
add a const `MAX_SCRIPTNUM_VALUE`.
74f3a5aeda Run clippy from the test script (Tobin C. Harding)
aa8109a791 Use struct field short form (Tobin C. Harding)
d1a05401f4 Remove redundant calls to clone (Tobin C. Harding)
196492554d Use assert instead of assert_eq (Tobin C. Harding)
3173ef9dbb Remove unnecessary explicit reference (Tobin C. Harding)
Pull request description:
Currently we run clippy in CI using a github action. The invocation has a couple of shortcomings
1. it does not lint the tests (this requires `--all-targets`)
2. it does not lint the examples
I could not find a way to lint the examples without explicitly linting each example by name.
**This PR does the following:**
- Fix clippy issues (patch 1-4)
- Move the clippy control to `test.sh`
- Add an env var `DO_LINT` to control it
- Remove the separate CI job
- Run the linter during the `Test` job using the stable toolchain.
- Run clippy with ` --all-features` AND `--all-targets` (only recently made possible).
- Run each example explicitly
Thanks to dunxen for noticing all the errors in my psbt example on review and prompting me to work out why clippy was not running :)
ACKs for top commit:
apoelstra:
ACK 74f3a5aeda
Kixunil:
ACK 74f3a5aeda
Tree-SHA512: 56dc6262144f4caa5efa6fdc46aeecf7bddc050ef134a639b31a34d4c5e01abcdeda18a00f4ded443866bbdfc982b4e5b67b0089639e0c253e207f0b54777f57
Clippy emits two warnings of form:
warning: redundant field names in struct initialization
Remove the redundant field names and use struct short initialization
form.
Clippy emits various warnings of type:
warning: this expression creates a reference which is immediately
dereferenced by the compiler
As suggested, remove the unnecessary explicit reference.
ee29910911 BIP152: Test net msg ser/der and diff encoding (0xb10c)
cd1aaaf344 BIP152: Add Compact Block unit test based on Elements (Steven Roose)
d4d92a838e BIP152: Add Compact Blocks network messages (Steven Roose)
f2fcdc86e6 BIP152: Add basic Compact Block structures (Steven Roose)
a9a39c4b08 blockdata: Derive PartialOrd, Ord and Hash for BlockHeader (Steven Roose)
Pull request description:
> Adds the basic structures for BIP152 and a method to create a compact block from a full block.
This is a rebase of #249 by stevenroose (see https://github.com/rust-bitcoin/rust-bitcoin/pull/249#issuecomment-1170562141) with a milestone for 29.0. I've added deserialization and serialization tests for the network messages and fixed an off-by-one bug in the deserialization of the differentially encoded varints in the `getblocktxn` message (see https://github.com/rust-bitcoin/rust-bitcoin/pull/249#discussion_r914989521).
Closes#249.
ACKs for top commit:
tcharding:
ACK ee29910911
apoelstra:
ACK ee29910911
Tree-SHA512: 462a91576281f5a2ffdc2610769ea93970b60dac75a150c827966c48daec7cf93f526f9f202e7ba1dbb1410b49148579880260a3c3df298b98330c0d891a4cca
3c2869465b Use to_be_bytes (Tobin C. Harding)
Pull request description:
Now that MSRV is > 1.32 we can use `u16::to_be_bytes` to ensure network byte order when encoding the port number of a `AddrV2Message`.
Remove the TODO and use `to_be_bytes` as suggested.
ACKs for top commit:
Kixunil:
ACK 3c2869465b
apoelstra:
ACK 3c2869465b
Tree-SHA512: b5dd9b0f43257f84defb9e9ecf9d02469f1959669367c5ad02cfb43003b780cf07ea344579b52a75c424fd85f8fa02dee3713b967c9b3a33eec6b7aa914763cd
7f498ee2a2 Add docs re Rust 1.51.1 (Tobin C. Harding)
Pull request description:
After auditing the code base for 'msrv' the only remaining mention (excl. md files) is on `unsigned_abs`.
Add docs to save the next guy looking up what version of Rust introduced `unsigned_abs`. (I also added an item to the [msrv tracking issue](https://github.com/rust-bitcoin/rust-bitcoin/issues/1060).)
ACKs for top commit:
Kixunil:
ACK 7f498ee2a2
apoelstra:
ACK 7f498ee2a2
Tree-SHA512: 9b4bdca09d3f7ac1c0584f4eb5207983a064cfe81ed26952d00396b2e0019ef40eee192b7f09d36c68b8c4e1f8de9cac2d1f3ee0946626bae089b46fe38704ab
Now that MSRV is > 1.32 we can use `u16::to_be_bytes` to ensure network
byte order when encoding the port number of a `AddrV2Message`.
Remove the TODO and use `to_be_bytes` as suggested.
517059e148 Use u8::try_from (Tobin C. Harding)
Pull request description:
Currently we use a cast to get the `u8` depth, as suggested by the TODO in the code we can now use `TryFrom` since we bumped the MSRV.
Use `u8::try_from.expect("")` since depth (node count) is guarded by `TAPROOT_CONTROL_MAX_NODE_COUNT`.
ACKs for top commit:
Kixunil:
ACK 517059e148
apoelstra:
ACK 517059e148
Tree-SHA512: 98e58617247a05025ec13ad3d00a464cb2351c98c6d871be43b252d3982e291b7187e25780e7fe367f5a3a64fa20f3b328876a8901af4da09e675f50727a26cf
This adds tests for serialization of BIP152 network messages and
tests specifically for the differential encoding of the transaction
indicies of 'getblocktx'. Previously, this code contained an
off-by-one error.
Currently we use a cast to get the `u8` depth, as suggested by the TODO
in the code we can now use `TryFrom` since we bumped the MSRV.
Use `u8::try_from.expect("")` since, assuming the code comment is
correct, the depth is guaranteed to be less that 127.
b1faf63e82 Use listener.accept() (Tobin C. Harding)
Pull request description:
During test network simulation we only accept a single connection, we can simplify the code by using `accept`.
Done as a follow up to review suggestion:
https://github.com/rust-bitcoin/rust-bitcoin/pull/1042#discussion_r898013799
ACKs for top commit:
Kixunil:
ACK b1faf63e82
apoelstra:
ACK b1faf63e82
Tree-SHA512: b2ead15d3108db3e01d9faab5e3521403dad6a0f4c3cf505f88fefd020110c520a89b9406484c10b04c9a34073c8abc465941577b17b5a193b54502c23b14c61
Replace all instances of
`secp256k1::Message::from_slice(_).expect(_)` with
`secp256k1::Message::from(_)`.
Also adds an implementation of ThirtyTwoByteHash for
TapSighashHash.
Solves https://github.com/rust-bitcoin/rust-bitcoin/issues/824
For internal macros used only in this crate we do not need to use
`macro_use` and pollute the top level namespace now that we have edition
2018. We can add a `pub(crate) use` statement to each and then path
imports work for the macros like normal types.
Now we have edition 2018 we do not need to use `macro_use` or `extern
crate`; `pub use` works with macros. Notable exceptions are `alloc` and
`test`. Also leave the serde rename because touching it opens a can of
worms.
Recently we removed the "unstable" feature, I missed the duplicate
`extern crate test` when doing so :(
Since we no longer have the "unstable" feature this line of code is
never compiled in.
Remove the unused ``extern crate test`, we have the correct line further
up the file `#[cfg(bench)] extern crate test;`.
Now we have edition 2018 we do not need to use `macro_use` or `extern
crate`; `pub use` works with macros.
Remove the extern crate statement and replace it with a pub use
statement. Requires fixing up various imports statements and also
requires importing `sha256t_hash_newtype` macro directly instead of
using a qualified path to it.
In preparation for running the formatter on `src/` and a function local
use statement for `$crate::serde:🇩🇪:Unexpected`, this shortens the
line of code that uses this type preventing the formatter for later
munging that line.
The local variable `formatter` can be shortened to `f` with no loss of
clarity since it is so common.
Done in preparation for running `rustfmt` on `src`.
In preparation for running the formatter on all source files at the root
of the crate; skip formatting `prelude` module because we use
unconventional import statements so they to save writing a million
compiler attributes.
In preparation for running the formatter on root level modules and a
submodule that holds all the macro calls in `hash_types` so we can
configure rustfmt to skip formatting them.
73bc2bb058 Remove leading colons from ::core::cmp::Ordering (Tobin C. Harding)
bffe0e840d Remove _most_ leading double colons (Tobin C. Harding)
Pull request description:
Leading double colons are a relic of edition 2015. Attempt to remove _all_ leading double colons (assuming I didn't miss any).
- Patch 1 is done mechanically so it can be repeated by reviewers, just search-and-replace ' ::' with '::' (note the leading space).
- Patch 2 does a single other instance of leading `::`
ACKs for top commit:
apoelstra:
ACK 73bc2bb058
sanket1729:
utACK 73bc2bb058.
Tree-SHA512: 8f7aafdda1aed5b69dcc83f544e65085dfec6590765839f0a6f259b99872805d1f00602fd9deac05c80a5cac3bc222d454dde0117dff4484e5cd34ea930fdfa1
a1df62a3d9 Witness human-readable serde test (Dr Maxim Orlovsky)
68577dfb50 Witness human-readable serde (Dr Maxim Orlovsky)
93b66c55b3 Witness serde: test binary encoding to be backward-compatible (Dr Maxim Orlovsky)
b409ae78a4 witness: Refactor import statements (Tobin C. Harding)
e23d3a815c Remove unnecessary whitespace (Tobin C. Harding)
ac55b1017e Add whitespace between functions (Tobin C. Harding)
Pull request description:
This is dr-orlovsky's [PR](https://github.com/rust-bitcoin/rust-bitcoin/pull/899) picked up at his permission in the discussion thread.
I went through the review comments and implemented everything except the perf optimisations. Also includes a patch at the front of the PR that adds a unit test that can be run to see the "before and after", not sure if we want it in, perhaps it should be removed before merge.
This PR implicitly fixes 942.
To test this PR works as advertised run `cargo test display_transaction --features=serde -- --nocapture` after creating a unit test as follows:
```rust
// Used to verify that parts of a transaction pretty print.
// `cargo test display_transaction --features=serde -- --nocapture`
#[cfg(feature = "serde")]
#[test]
fn serde_display_transaction() {
let tx_bytes = Vec::from_hex(
"02000000000101595895ea20179de87052b4046dfe6fd515860505d6511a9004cf12a1f93cac7c01000000\
00ffffffff01deb807000000000017a9140f3444e271620c736808aa7b33e370bd87cb5a078702483045022\
100fb60dad8df4af2841adc0346638c16d0b8035f5e3f3753b88db122e70c79f9370220756e6633b17fd271\
0e626347d28d60b0a2d6cbb41de51740644b9fb3ba7751040121028fa937ca8cba2197a37c007176ed89410\
55d3bcb8627d085e94553e62f057dcc00000000"
).unwrap();
let tx: Transaction = deserialize(&tx_bytes).unwrap();
let ser = serde_json::to_string_pretty(&tx).unwrap();
println!("{}", ser);
}
```
Fixes: #942
ACKs for top commit:
apoelstra:
ACK a1df62a3d9
Kixunil:
ACK a1df62a3d9
Tree-SHA512: d0ef5b8cbf1cf8456eaaea490a793f1ac7dfb18067c4019a2c3a1bdd9627a231a4dd0a0151a4df9af2b32b909d4b384a5bec1dd3e38d44dc6a23f9c40aa4f1f9
e34bc538c3 Add new type for sequence (Noah Lanson)
Pull request description:
#1082
Created a new type for txin sequence field with methods to create sequences with relative time locks from block height or time units.
ACKs for top commit:
Kixunil:
ACK e34bc538c3
tcharding:
ACK e34bc538c3
apoelstra:
ACK e34bc538c3
Tree-SHA512: 6605349d0312cc36ef9a4632f954e59265b3ba5cfd437aa88a37672fe479688aa4a3eff474902f8cc55848efe55caf3f09f321b3a62417842bfc3ec365c40688
f3b2120ec9 Create configuration conditional bench (Tobin C. Harding)
f60c92ca58 Add informative error message to DO_BENCH (Tobin C. Harding)
c6d5a12b60 Add cargo/rustc sanity calls (Tobin C. Harding)
34d5a3141d Put triple ticks on their own line (Tobin C. Harding)
Pull request description:
Currently we are unable to build with all features enabled with a non-nightly toolchain, this is because of the use of
`#![cfg_attr(all(test, feature = "unstable"), feature(test))]`
which causes the following error when building:
error[E0554]: `#![feature]` may not be used on the stable release channel
The "unstable" feature is used to guard bench mark modules, this is widely suggested online but there is a better way.
When running the bench marks use the following incantation:
`RUSTFLAGS='--cfg=bench' cargo bench`
This creates a configuration conditional "bench" that can be used to guard the bench mark modules.
```
#[cfg(bench)]
mod benches {
...
}
```
ACKs for top commit:
Kixunil:
ACK f3b2120ec9
apoelstra:
ACK f3b2120ec9
Tree-SHA512: 7ec2a501a30bfe2ce72601077cd675cf5e5ac2f0f93f97fc7e83cb7401606b69ae909b35bfc0ace8bd1ea771ca4fba70e2ad9ac9ba26f2b6e371494cf694c0a8
5ce34011f2 Implement std::error::Error for ParseAmount (Tobin C. Harding)
Pull request description:
The `ParseAmountError` does not implement `std::error::Error`, must have been missed when we did the rest.
Implement `std::error::Error` for `ParseAmount`.
ACKs for top commit:
Kixunil:
ACK 5ce34011f2
apoelstra:
ACK 5ce34011f2
Tree-SHA512: 8dafc472b7c23b54d856344e786e0f22e8e179f30f6c1011fbf5f8f0c6b1b5d74ed8e4f2638e5f8246f04dbb429e60027db6fe584d51a78957a6e904feb9e8a3
Currently we are unable to build with all features enabled with a
non-nightly toolchain, this is because of the use of
`#![cfg_attr(all(test, feature = "unstable"), feature(test))]`
which causes the following error when building:
error[E0554]: `#![feature]` may not be used on the stable release
channel
The "unstable" feature is used to guard bench mark modules, this is
widely suggested online but there is a better way.
When running the bench marks use the following incantation:
`RUSTFLAGS='--cfg=bench' cargo bench`
This creates a configuration conditional "bench" that can be used to
guard the bench mark modules.
#[cfg(bench)]
mod benches {
...
}
1e46eeaa88 Upgrade to bitcoin_hashes v0.11.0 (Tobin C. Harding)
Pull request description:
We just released a new version of `bitcoin_hashes`, upgrade the dependency to v0.11.0
ACKs for top commit:
apoelstra:
ACK 1e46eeaa88
Kixunil:
ACK 1e46eeaa88
Tree-SHA512: 32173349c280c255681ebf15a1aa98a28b7016bb813ca907ac55a9797d9d17e1da344e2d8e74c02c2c80d2e6873a03f522c2d55b289b65d7ac55f402b19d689b
24f0441d54 Add PublicKey::to_sort_key method for use with sorting by key (junderw)
Pull request description:
Replaces #524
See previous PR for reasoning.
This solution is a little more straightforward. The name and documentation should be enough to prevent misuse.
We can also impl a to_sort_key for any CompressedKey added later. (or just impl Ord in a BIP67 compliant way)
TODO:
- [x] Add more sorting test vectors. Ideas of edge cases to test are welcome.
ACKs for top commit:
apoelstra:
ACK 24f0441d54
tcharding:
ACK 24f0441d54
Kixunil:
ACK 24f0441d54
Tree-SHA512: 92d68cccaf32e224dd7328edeb3481dd7dcefb2f9090b7381e135e897c21f79ade922815cc766c5adb7ba751b71b51a773d103af6ba13fc081e1f5bfb846b201
Previous implementations of Witness (and Vec<Vec<u8>>) serde serialization
didn't support human-readable representations. This resulted in long unreadable
JSON/YAML byte arrays, which were especially ugly when pretty-printed (a line
per each byte).
Co-authored-by: Tobin C. Harding <me@tobin.cc>
This also removes tests for JSON backward-compatible encoding. Human-readable
encoding will be changed in the next commit and this will break backward
compatibility, thus that part of the test is removed.
35edcaa4ca Remove extern crate core statement (Tobin C. Harding)
Pull request description:
Now that we have an MSRV of 1.41.1 we no longer need `extern crate core`, remove it.
ACKs for top commit:
apoelstra:
ACK 35edcaa4ca
Kixunil:
ACK 35edcaa4ca
Tree-SHA512: 0efa0f05d3e9f797c493757fce6ca7703dd8439dbce4e9ff8113494fc71691ab4171c6f0fb5239b57ff5f0e223d48b057265a3df79898a399ca455040580aab2
36f29d4357 Upgrade to secp256k1 v0.23.0 (Tobin C. Harding)
Pull request description:
We recently released a new version of `rust-secp256k1`, upgrade to use it.
ACKs for top commit:
apoelstra:
ACK 36f29d4357
Kixunil:
ACK 36f29d4357
Tree-SHA512: 46a909dec8bc59daa78acdb76824d93f4f1da0e9736cf6ca443d3bbadfa43867e720293bb7c4919cb0658e75ec59daeffea080611f0e7eed4df439ddac0305de
19ba7ecc03 Add custom error for unknown address type parsing (Arturo Marquez)
Pull request description:
Adds a custom error `UnknownAddressType(_)` which is returned in cases where an unknown address type tries to be parsed via
`FromStr`. This provides more context for the error, since it contains the `String` that tried to be parsed.
Closes https://github.com/rust-bitcoin/rust-bitcoin/issues/1064
ACKs for top commit:
Kixunil:
ACK 19ba7ecc03
dunxen:
ACK 19ba7ec
tcharding:
ACK 19ba7ecc03
apoelstra:
ACK 19ba7ecc03
Tree-SHA512: 2f94eb2e122c1acafcb8c852f7bfe22cb3725806541ae40f82d3a882011fb911ce8fc430153ebea4066f43c5a51813359a4c9b95d2a9077ce8f6dcaa58eee6fd
Martin Habovstiak
Tobin C. Harding
Andrew Poelstra
0xb10c
Steven Roose
Arturo Marquez
Noah Lanson
Dr Maxim Orlovsky