We have introduced `rustfmt` but forgot to update the docs section about
it. Since a large portion of the codebase is currently ignored by our
`rustfmt` configuration, point out that `rusntfmt` is work in progress.
We have docs that use an env var `BITCOIN_MSRV` that no longer exists.
Point readers towards `RUSTUP_TOOLCHAIN`.
(Note that we still use `TOOLCHAIN` in the script but it is being phased
out, `RUSTUP_TOOLCHAIN` works today.)
We have stale docs referring to the old MSRV. We do not need MSRV docs
in `CONTRIBUTING` because we have them in the README already.
Fix stale docs by doing:
- Remove the MSRV docs from readme in favour of `CONTRIBUTING.md`.
- Add a sentence to the redame pointing readers towards `CONTRIBUTING.md`.
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.
Move the clippy control to `test.sh` and add an env var `DO_LINT` to
control it. Remove the explicit CI job and run the linter during the
`Test` job using the stable toolchain.
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.
cda097dda8 Add ci check for duplicate dependencies (Tobin C. Harding)
Pull request description:
Add a call to `cargo tree --duplicates` in the ci script to ensure that we do not have any duplicated dependencies.
Kudos to Kixunil for the idea (over in: https://github.com/rust-bitcoin/rust-bitcoin/pull/1104)
ACKs for top commit:
apoelstra:
ACK cda097dda8
Kixunil:
ACK cda097dda8
Tree-SHA512: 77f07dd5c6794b5a59293bd62bda0fe61384a30cf8258e79aca9ce32090f869f0a13929b6a7a4c35e10fc653968b12ddd4c291df9ecd0962632017f59c81d025
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
21a1cc791c Use pub(crate) for macros instead of macro_use (Tobin C. Harding)
23ee0930c7 Remove extern crate (Tobin C. Harding)
da8b1b5439 Remove unused extern crate test (Tobin C. Harding)
01a8cc6848 Remove extern crate bitcoin_hashes (Tobin C. Harding)
Pull request description:
Do some clean up of `extern crate` statements now we have edition 2018.
4 separate but very similar patches, separated to ease review.
ACKs for top commit:
sanket1729:
utACK 21a1cc791c
apoelstra:
ACK 21a1cc791c
Tree-SHA512: fba33ed8fd261cc756dad8dd94f186a5b38aaf20cf31c3a83ad7633e7bb60a390681c39ebfd913e9e242fffed3b502491d067250d72ebfe666b4d03e93c8b945
df73576515 Upgrade to secp v0.24.0 (Tobin C. Harding)
Pull request description:
We just released a new version of `rust-secp256k1`, lets use it.
This also fixes a bug where we upgraded our `bitcoin_hashes` dependency
before secp had done theirs.
ACKs for top commit:
sanket1729:
utACK df73576515
apoelstra:
ACK df73576515
Tree-SHA512: 614eb096203753a6581e444aa11d41c7060342afd7ad103f6118c64bccf604850819ec2852730d1ac83fdea193770fe7600ea70c082a19afaef7ad3e8016d0e4
c1d74a3eae Run the formatter (Tobin C. Harding)
Pull request description:
We recently merged a PR that enables the formatter on the `examples/` directory, at the same time we merged `examples/ecdsa-psbt.rs` but it had formatting issues under the new formatter config.
Run `cargo +nightly fmt` to format the `examples/` directory.
ACKs for top commit:
sanket1729:
utACK c1d74a3eae
apoelstra:
ACK c1d74a3eae
Tree-SHA512: 645bf01c7cbb21e2eec23e6fa9c2f5d26e03566059d906e1985af5a9c0186470ef8a0c295f7be0a5226e2f948e2a94aa336de4b3704a31fd8715764f1625737d
We just released a new version of `rust-secp256k1`, lets use it.
This also fixes a bug where we upgraded our `bitcoin_hashes` dependency
before secp had done theirs.
We recently merged a PR that enables the formatter on the `examples/`
directory, at the same time we merged `examples/ecdsa-psbt.rs` but CI
had already run so the formatter was not run.
Run `cargo +nightly fmt` to format the `examples/` directory.
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.
ded1a32a59 Add cargo crev reminder to readme (Tobin C. Harding)
Pull request description:
As suggested by the `cargo-crev` project; add a comment to the readme reminding people to use `cargo-crev` to check their dependencies.
### Notes
Today I explored `cargo-crev`, it was new to me before today. I completed proofs for `bech32`, `rust-bitcoinconsenus`, `bitcoin_hashes`, `rust-secp256k1`, and `rust-bitcoin`. I published the proofs to https://github.com/tcharding/crev-proofs.
If I'm understanding correctly proofs are only useful if the author is connected to a web of trust. So far I only found @dpc within the active rust-bitcoin devs with a `crev-proofs` repo (that includes an ID). Since he wrote `cargo-crev` its not surprising he has one :) Two other devs have `crev-proofs` repos but they are both incomplete (no ID) so I was unable to climb onto their web, so to speak. I am not a particularly well know dev so I imagine it would be more useful if some of you more well know fellas publish proofs as well.
If we can get a web of trust between all the regular hackers here then we can start doing reviews/proofs of our dependencies and publishing them.
ACKs for top commit:
Kixunil:
ACK ded1a32a59
apoelstra:
ACK ded1a32a59
sanket1729:
ACK ded1a32a59
Tree-SHA512: c2d3b195a522095fcabcf51bb956b339f3a421541652f646f8e56286ebf850aa106d4acbf4defd344b5b0f57dd9626d1dbafe50c9d54b1436fd9e2c8b434fc07
ee9a3ec6a1 Enable formatter for "src" (Tobin C. Harding)
743b197124 Add use for Unexpected (Tobin C. Harding)
fd217e72c3 Use f instead of formatter (Tobin C. Harding)
7b50a96ade Do not format prelude module (Tobin C. Harding)
01471f7e65 Shield hash_newtypes from rustfmt (Tobin C. Harding)
65d19d9044 Refactor compile_error message (Tobin C. Harding)
6461e2db8d Run formatter on examples/ (Tobin C. Harding)
fd1c6589ba Add a rustfmt configuration file with explicit ignores (Tobin C. Harding)
Pull request description:
Looks like we are getting to a place that rustfmt _might_ be able to be used. In an effort to introduce `rustfmt` without requiring devs to do excessively mundane reviews introduce `rustfmt` in a non-invasive manner. What this means is
- Add a fully fleshed out `rustfmt` config file that explicitly ignores all the source files
- Enable sections of code one by one so review is easier (including preparatory patches before doing each section).
This PR currently does `examples` and all the source files at the root level of the crate (i.e. excludes all directories in `src/`). The other directories can then be done one at a time.
Please see discussion on: https://github.com/rust-bitcoin/rust-bitcoin/pull/959 for more context.
ACKs for top commit:
sanket1729:
ACK ee9a3ec6a1
apoelstra:
ACK ee9a3ec6a1
Tree-SHA512: f4ff3c031b5d685777e09bac2df59ed8217576b807da7699f44fe00aa509d0b7fe47617a8b3eff00d3125aeece3e010b8f9dd8733d315ccb2adc0e9a7d7f07e3
cd2369b4f9 Run ecdsa-psbt example in test script (Tobin C. Harding)
6967c0ed6f Add psbt example (Tobin Harding)
Pull request description:
Add an example PSBT workflow. The workflow we simulate is that of a setup using a watch-only online wallet (contains only public keys) and a cold-storage wallet (contains the private keys).
We create and update a PSBT using the watch-only wallet then pass the PSBT to the cold-storage wallet to sign.
Partially resolves#892 (more done in #935).
## Note
This PR includes a sub-module in `examples/psbt.rs` that implements ECDSA signing. This will hopefully eventually be merged into the main crate by way of https://github.com/rust-bitcoin/rust-bitcoin/pull/957. We have three PRs that add examples/tests of PSBTs that need to do signing, in order to help us design a good AP in #957 I think it would be beneficial to complete and merge these three PRs.
1. This PR (PSBT ECDSA example)
2. PBST ECDSA integration test: https://github.com/rust-bitcoin/rust-bitcoin/pull/935
3. PSBT taproot example: https://github.com/rust-bitcoin/rust-bitcoin/pull/999
Note to self, this will need a change to `test.sh` if #1079 merges.
ACKs for top commit:
apoelstra:
ACK cd2369b4f9
sanket1729:
ACK cd2369b4f9. The example is clean, you can address the comments in followups.
Tree-SHA512: c4fb8ec631bf8bfc30534e8974b1f6c4bb7cc6def165a4ee2bb7aa73f5aa7fdc11d2000ca25792a4b534b2c5bf1592efe542ada14a9d702c7dfacbaa808f3aea
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.
Add a configuration file but explicitly ignore all source files. The aim
of this is that we can then un-ignore files a few at a time and deal
with issues in a small isolated manner.
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
Add an example PSBT workflow. The workflow we simulate is that of a
setup using a watch-only online wallet (contains only public keys) and a
cold-storage wallet (contains the private keys).
We create and update a PSBT using the watch-only wallet then pass the
PSBT to the cold-storage wallet to sign.
Co-authored-by: Dan Gould <d@ngould.dev>
Tobin C. Harding
Andrew Poelstra
0xb10c
Steven Roose
Arturo Marquez
sanket1729