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.
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>
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
ef7fef001c Derive Hash on a bunch of types (Tobin C. Harding)
Pull request description:
In preparation for being able to derive `Hash` on all types in `miniscript`, derive `Hash` on all of the required types.
This PR includes all the changes in https://github.com/rust-bitcoin/rust-bitcoin/pull/933/files and hence supersedes it.
ref: https://github.com/rust-bitcoin/rust-miniscript/issues/226
ACKs for top commit:
Kixunil:
ACK ef7fef001c
apoelstra:
ACK ef7fef001c
Tree-SHA512: 1a1db8b4df2ea8f9e176434bb6fdee5b96f47dcdc6395ebc59e5f5ac5eb13a66fb61e1d90cdbbf12a027f7685fdff21060338c5f27b9d9bf5e9fee452c7c7e83
896ca42a53 Document PSBT roles and limitation (DanGould)
Pull request description:
The READEME claims rust-bitcoin supports PSBT finalization, but really needs rust-miniscript for that. I think we should make this clear in this crate's PSBT examples as well.
> > Understanding scripts and witnesses doesn't scream rust-miniscript to me. Miniscript crate is not the first place I'd look, since it's additional.
>
> We should probably add it to the README here or something. If I needed to deal with scripts or witnesses in any way beyond reserializing them or hashing scriptpubkeys into addresses, I'd immediately turn to Miniscript. You basically can't work on scripts without it, unless you are doing something very specific.
>
> >Anywhere there's PSBT code I expect a finalizer, even if it had documented limitations.
>
> Not to be too glib, but suppose we had a finalizer that just always returned an error, and was documented not to finalize any transactions. Would that meet your expectations? It is hard to do much more without Miniscript.
>
_Originally posted by apoelstra in https://github.com/rust-bitcoin/rust-bitcoin/issues/630#issuecomment-1175316689_
ACKs for top commit:
apoelstra:
ACK 896ca42a53
tcharding:
ACK 896ca42a53
Tree-SHA512: e71a65b8c04134d9b3406ea76bb915fa116e4a961f9f6cb24350422f9d550cba26a630e02f9ba352fae63076926532bc4bf2d1001488666a05f18d7774ddda9e
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 {
...
}
If CI script is run with `DO_BENCH=true` and `TOOLCHAIN` set to a
non-nightly toolchain the build will fail with a less than meaningful
error. To assist runners of the script output an informative error
message if an attempt is made at using the wrong toolchain.
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
In markdown triple ticks go on a line of their own. This change does not
effect the rendering in GitHub which is managing to parse this section
correctly already.
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
91ff2f628c Introduce SPDX license identifiers (Tobin C. Harding)
Pull request description:
When `rust-bitcoin` was started in 2014 the SPDX license list and short identifiers where not a thing. Now that we have short identifiers and they are gaining popularity in other projects we can consider using them.
- Add links to the SPDX website in the readme
- Shorten the author section to a single line
- Remove all the licence information in each file and replace it with an
SPDX ID (see https://spdx.dev/ids/#how)
Of note:
- If the author of a file is explicitly listed, maintain this information
- If the 'author' is listed as the generic 'Rust Bitcoin developers' just remove the attribution, this is implicit. This does loose the date info but that can be seen at any time from the git index using
`git log --follow --format=%ad --date default <FILE> | tail -1`
apoelstra, please confirm that I'm not treading on your toes here, especially, are you ok with the new 'written by' string format?
### Ref
- https://spdx.dev/ids/#how
- https://spdx.org/licenses/CC0-1.0.html
- https://spdx.dev/ids/
ACKs for top commit:
apoelstra:
ACK 91ff2f628c
sanket1729:
ACK 91ff2f628c. I am also in IDGAF camp, but I like more red lines in diff.
Kixunil:
ACK 91ff2f628c
Tree-SHA512: ca8aac00f015c18ec18de83dfeb50dd6f4f840653c7def85daa2436a339021ada5f3c34ad0cdf6b18e3e39c45a6d58a8313742e4001d467785b10eee7fdbc938
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.
For more info see [1].
[1] - `https://github.com/rust-bitcoin/rust-bitcoin/issues/1064`
Leading double colons are a relic of edition 2015. Remove all leading
double colons that follow a space, done like this so that reviewers can
do the same and verify the diff. Done with
search-and-replace ' ::' '::'
And, for the record:
```bash
function search-and-replace() {
if (($# != 2))
then
echo "Usage: $0 <this> <that>"
return
fi
local this="$1"
local that="$2"
for file in $(git grep -l "$this")
do
perl -pi -e "s/$this/$that/g" "$file"
done
}
```
Andrew Poelstra
Tobin C. Harding
sanket1729
Noah Lanson
DanGould
Dr Maxim Orlovsky
Arturo Marquez