596e756d01 Pin syn dependency for MSRV toolchain (Tobin C. Harding)
Pull request description:
CI change only, can this merge without 2 acks?
The recent 1.0.108 `syn` update violated our MSRV, pin `syn` in the CI script.
ACKs for top commit:
apoelstra:
ACK 596e756d01
Tree-SHA512: b27d8c18eae36025ac86bbad8f6a8bab3fccc8263e45b7421f216a0b1c6c318e02588c67094d8bd69a23efb32da9dfd10c2a6ce2bd5435133613a38906d83a38
438ee45691 Show cache construction in rustdoc (Tobin C. Harding)
Pull request description:
To make it more clear what the cache is show the cache construction line in rustdoc.
ACKs for top commit:
apoelstra:
ACK 438ee45691
Kixunil:
ACK 438ee45691
Tree-SHA512: d6da6bad57fddf9e2f4bcfb7c9b87df38bf4b2bb914e92e52d5ae8afa3405a9793536d7164223021ab6d183ddde732cf6889370834e36f37bae470127b0271fa
2620f3b69d Put optional = true at the end (Tobin C. Harding)
d86ef3b01b Put non-optional dependency with other non-optional (Tobin C. Harding)
Pull request description:
Late stage, super anal, manifest cleanup.
- Patch 1: put non-optional dependencies together
- Patch 2: put `optional = true` at the end of the line
ACKs for top commit:
apoelstra:
ACK 2620f3b69d
Kixunil:
ACK 2620f3b69d
Tree-SHA512: bca2bd2eb9a50aea04cab7c6eab7c264e91937ff20473ccf4496ba380897d1d50696719aa3379bd3f29b8a3cbe43d5485ac068f0e2c126462f361fcfe9941548
f69363d71a adding suggested documentation for path arg (Lorenzo Maturano)
673ca2d2fe changing docs and examples to use reference to slice in `derive_pub` (Lorenzo Maturano)
Pull request description:
`P: AsRef<[ChildNumber]>` means that path can be borrowed as a reference to a slice of `ChildNumber`.
While it does work with `Vec`, the documentation and examples mislead the user into thinking that he/she has to use a vec, which is not true. A simple reference to a slice of `ChildNumber` also works and does not consume heap
ACKs for top commit:
tcharding:
ACK f69363d71a
Kixunil:
ACK f69363d71a
Tree-SHA512: 9f0a2764c676ecaa45de69663c83c3008ee01f534c324ceca627c69d689d6ca6d0a3dda9361eaa402bacc68975e998840b78a3856e0572db3de10325b354ac6a
The API guidelines say macro input should be evocative of the output.
`hash_newtype` didn't have this property.
This change makes it look exactly like the resulting struct, `$len`
parameter was removed since it's not needed, reversing is controlled
using an attribute. The macro is also better documented and ready to be
extended in the future.
The tagged SHA256 newtype is not yet modified because it has a more
complicated input parameters.
Closes#1648
d83739a980 Clarify the intention of strange condition (Martin Habovstiak)
Pull request description:
It may not be obvious why the condition in `push_bytes` module checks for negation of 16 and 32 bit architectures rather than 64 bit. This adds a comment about it being conservative.
Addresses https://github.com/rust-bitcoin/rust-bitcoin/pull/1190#discussion_r1111342671
ACKs for top commit:
apoelstra:
ACK d83739a980
tcharding:
ACK d83739a980
Tree-SHA512: 8a674b9869c580a007c189b0d9b0a57023b26eff3afab25322d966e0ccaff767d19dd499243d438832972f351e1e7ea4b8e387a5e3c74569816814fea258b20b
It may not be obvious why the condition in `push_bytes` module checks
for negation of 16 and 32 bit architectures rather than 64 bit. This
adds a comment about it being conservative.
6fb2d12373 Get rid of BadFormat error (hashmap)
Pull request description:
add additional variants instead.
as discussed in https://github.com/rust-bitcoin/rust-bitcoin/pull/1365
ACKs for top commit:
Kixunil:
ACK 6fb2d12373
apoelstra:
ACK 6fb2d12373
Tree-SHA512: 2cf9146670c372a3a482448f84a30943cd2ff2fa4e724075d67a52dba5ac0ad38f99ca2af3dd3494e13584653f2b23e913e6421328d40be52e868a107fffe03b
be7b3754a9 Rename schnorr module to taproot (Tobin C. Harding)
9f39e872bc Rename SchnorrSighashType to TapSighashType (Tobin C. Harding)
f5c26693c5 Make match arms more terse (Tobin C. Harding)
40c246743b Split Sighash into LegacySighash and SegwitV0Sighash (Tobin C. Harding)
e38d843536 Do not use deprecated function in rustdoc example (Tobin C. Harding)
98130f49f1 Rename TapSighashHash to TapSighash (Tobin C. Harding)
7e4da3c0ab Move taproot keys to the keys module (Tobin C. Harding)
c5fe315a93 Move sighash to crypto module (Tobin C. Harding)
Pull request description:
This PR is now part 1 of great sighash clean up. It does not attempt to split ecdsa into two parts as discussed below, that is WIP and will be done in a separate PR after this upcoming release. Does however create `LegacySighash` and `SegwitV0Sighash` types.
This PR moves us along the road by doing:
- Move `sighash` to the `crypto` module, where I should have put it ages ago :)
- Move the taproot keys to the `keys` module
- Rename the `schnorr` module -> `taproot`
- Rename `TapSighashHash` -> `TapSighash`
- Rename `SchnorrSighashType` -> `TapSighashType`
- Fix a bunch of other schnorr usage (including pub/priv methods).
This leaves us in a situation where we have taproot sig and sighash type, and non-taproot ones prefixed with "ecdsa". Its not uniform but it kind of makes sense.
Fix: #1542Fix: #1550
ACKs for top commit:
Kixunil:
ACK be7b3754a9
apoelstra:
ACK be7b3754a9
Tree-SHA512: 4c52bea9ba71713afa181d625153d73d3c5b7c6f5786248960958982332a6a79501cd26e13a66bfb9700d9b8f59717446abb795ecbfbd8d60d9d59a38c1d718b
"schnorr" is a dirty word; the current `schnorr` module defines a
`Signature` that includes a sighash type, this sighash type is a bitcoin
specific construct related to taproot. Therefore the `Signature` is
better named `taproot::Signature`. Note also that the usage of `schnorr`
in `secp256k1` is probably justified because the
`secp256::schnorr::Signature` is just doing the crypto.
While we are at it, update docs and error messages to use "taproot"
instead of "schnorr". Also change function names and identifiers that
use "schnorr".
Currently we have `TapSighash` that is used for taproot sighashes but
for non-taproot sighashes we use `hash_types::Sighash`. We can improve
the API by creating a `LegacySighash`, and `SegwitV0Sighash`.
Copy the original `Sighash` macro calls to create the two new types in
the `sighash` module.
While we are at it, put the `TapSighash` and `TapSighashTag` into the
`sighash` module also.
There is never any use for the `sighash` module unless one is signing,
which requires the `crypto` module. The `sighash` module should
therefore live in the `crypto` module. This is not an API breaking
change because we reexport it at the crate root.
bcd95fa036 Add a newtype for script-pushable slice (Martin Habovstiak)
8fedbcbf13 Add `ecdsa::SerializedSignature` (Martin Habovstiak)
26fc4152ec Use `PushDataLenLen` to improve confidence (Martin Habovstiak)
bb2c7ec790 Introduce `hex_lit` crate (Martin Habovstiak)
Pull request description:
The code previously contained undocumented panic when trying to push
slice. This is now solved by making a newtype that guarantees limited
length.
Closes#1186Closes#1189
This is done on top of unsized script change. I only request light review of the design for now. I'd like to improve it to use a macro for all unsized types.
ACKs for top commit:
tcharding:
ACK bcd95fa036
apoelstra:
ACK bcd95fa036
Tree-SHA512: 193bee9b72e850a1a827124e6774111dc9bbc41a3999ae777acbf201a2ca062cad96debb27b1fd3374c89bacb95aab22638e967b7a11b3cbe955f2d5521f64d9
2eb68caf81 Remove the rustfmt required_version config option (Tobin C. Harding)
Pull request description:
The `rustfmt` config option `required_version` causes grief in CI because everytime `rustfmt` is update our `nightly` job fails. Since we require nightly anyways to run `rustfmt` the config option is basically redundant.
ACKs for top commit:
apoelstra:
ACK 2eb68caf81
Kixunil:
ACK 2eb68caf81
Tree-SHA512: fcd4f8b13e22f7094fdea0f3fa5cf7644d27c17da59a65ab7c43bdbdb2f4c672834d10cb848814aaa9cc9c626fa21ff2e759ae25d3249d52f02d7969352d1443
`Signature` only supported serialization into `Vec` which required a
heap allocation as well as prevented statically proving maximum length.
Adding a specialized type that holds a byte array and size solves this.
The solution is very similar to `secp256k1::ecdsa::SerializedSignature`.
The difference is that serialized signature in this crate contains
sighash bytes flag while in `secp256k1` it doesn't.
Script parsing is composed of several functions which implicitly rely on
various properties. Adding a type that restricts the valid values makes
local review easier.
So far we deserialized hex into `Vec<u8>` at run time. This was mainly
in tests where it had negligible performance cost. However moving the
computation to compile time has a few benefits: it allows proving the
length of the decoded bytes and identifies potential typos before the
code goes through LLVM and other compilation machinery which makes
feedback faster.
This change uses the `hex_lit` crate to move computation to compile
time. It is implemented as `const` declarative macro which doesn't blow
up compilation time.
70bff0de8d ci: Remove incorrect code comment and cargo clean (Tobin C. Harding)
Pull request description:
Recently while trying to fix CI I (Tobin) added a call to `cargo clean` and a code comment justifying it. This was merged because while not incorrect it is redundant since calling `cargo` with different `RUSTFLAGS` triggers a rebuild.
ACKs for top commit:
apoelstra:
ACK 70bff0de8d
Kixunil:
ACK 70bff0de8d
Tree-SHA512: 5bed107aedf9d6d240fa885335bfe04099436c0d30c045554644b43a06900c415708dfcedbd27037564d296c0ab12e4c4d4dc1e25ab64b4cc07ca7b46c46628a
a121e19e94 hashes: Implement AsRef for fixed size arrays (Tobin C. Harding)
Pull request description:
Implement `AsRef<[u8; X]>` for hash types including wrapped hash types. Doing so means at times the compiler can no longer infer the type because we have `AsRef<[u8]` implemented also but we can use `into_inner` and `as_inner` to get the inner array if needed.
Fix: #1462
## Note
This touches code that will likely be changed by #1577 and when we do #1491 but I believe its a step forward.
ACKs for top commit:
arturomf94:
ACK [`a121e19`](a121e19e94)
apoelstra:
ACK a121e19e94
Kixunil:
ACK a121e19e94
Tree-SHA512: 257c44826c7649db25bb3a6f023f68b2f17b70c546a056afad044bc8a16bf61f654c3846222505aaf5e6f9a0ad1d2113272d61317b407d0ac83702e41060a1ee
c3cc9e52ab Fix absolute lock time examples and tests (Tobin C. Harding)
Pull request description:
An absolute lock time of 100 is nonsensical because we are well past block 100. This value was used because it makes sense for _relative_ locktimes but for absolute lock times it makes the examples and tests slightly confusing.
ACKs for top commit:
apoelstra:
ACK c3cc9e52ab
Kixunil:
ACK c3cc9e52ab
Tree-SHA512: f490ef111bce0989c4ce8300c507c21b454448af4a91b9ef7a2fc05407411ca8721c9caa3dd1f0e8c0c133c4892c5c512f2d881af2cc67ae843d87eacae76ef1
4a03e2e721 psbt: Remove unused error variant (Tobin C. Harding)
Pull request description:
Remove an unused error variant for PSBT code (API breaking because the error type is public).
Woops, somehow I managed to get what was patch 1 of this series merged yesterday, I thought I left it out. Anyways, this is just the remove unused error variant now. No changes to that patch from previous versions of the PR.
ACKs for top commit:
apoelstra:
ACK 4a03e2e721
Kixunil:
ACK 4a03e2e721
Tree-SHA512: 228c661b97c6656db5a2bcc9ceb494ea485363b7f7262a97c677ee1639b5209c92ec3715ff48fdb108c95c828bfc83b6c475aa66f0ce8c5b0f286bfa7cc19554
An absolute lock time of 100 is nonsensical because we are well past
block 100. This value was used because it makes sense for _relative_
locktimes but for absolute lock times it makes the examples and tests
slightly confusing.
dd316e4d14 pow: Remove Mul/Div by arbitrary integer types (Tobin C. Harding)
Pull request description:
When we added `Target` and `Work` types we implemented multiplication and division by anything `Into<u64>`, this is not typically done in the Rust stdlib and also is semantically incorrect for the types.
Remove `Mul` and `Div` impls from `Target` and `Work`. Also remove `Mul<T>` for `T: Into<u64>` from the private `U256` type.
Fix#1632
ACKs for top commit:
apoelstra:
ACK dd316e4d14
Kixunil:
ACK dd316e4d14
Tree-SHA512: ede53555844adab321ff344535b7b8bab3c5c73855823dfc3ad728b077ae199451b7e22a1d203ef73a076073b7f0cbf9637cefa5fe82fc78ab454d02fa0b62b9
a819cf99d6 Remove FromStr impl from ScriptBuf (Tobin C. Harding)
Pull request description:
`FromStr` impls should roundtrip with `Display` imlps but currently our `ScriptBuf` displays using instructions but parses hex.
Looks like this slipped back in during a recent rebase fail by me.
ACKs for top commit:
apoelstra:
ACK a819cf99d6
Kixunil:
ACK a819cf99d6
Tree-SHA512: 754f87147a1cffccea8b4016a4fe204b34e85a983dd89e4c3c2a73c694420f87fda6bea5b7a298a84e95a0e9b6942841fb5cdef0c12662b64819c79cfe5d96f9
When we added `Target` and `Work` types we implemented multiplication
and division by anything `Into<u64>`, this is not typically done in the
Rust stdlib and also is semantically incorrect for the types.
Remove `Mul` and `Div` impls from `Target` and `Work`. Also remove
`Mul<T>` for `T: Into<u64>` from the private `U256` type.
272cdbcf7c Flatten the types directory (Tobin C. Harding)
Pull request description:
We recently created a `types` subdirectory under `script` to keep all the `Script` and `ScriptBuf` impls together. Turns out this additional level of subdirectory is a bit annoying and we can achieve the same grouping by just using `script/mod.rs`.
Move code from `types/mod.rs` to `script/mod.rs`, move the two submodules up a level, remove the `types` directory.
Fix: #1640
ACKs for top commit:
Kixunil:
ACK 272cdbcf7c
apoelstra:
ACK 272cdbcf7c
Tree-SHA512: 91fd78084829fa24f3b6420602d7d5094670647fff43e6e193d6de3126f1657132873ea133540d87db7d0d4dfc4cb9666489e39c861377085ce0254da81fd564
9ae3285882 hashes: Improve feature gating (Tobin C. Harding)
Pull request description:
Currently we have a few things mixed up in the feature gating of `hashes`.
Observe that:
- `io::Write` and `io::Read` is not related to allocation but rather provided by `core2`
- "std" should be able to enable "alloc".
Improve feature gating by doing:
- Enable "alloc" from "std" and simplify `cfg` codebase wide.
- Enable "internals/alloc" from "alloc".
- Fix feature gating to use the minimal requirement i.e., "alloc".
Please note one anomaly, `internals` does not set "std" as the default feature but `hashes` does.
ACKs for top commit:
Kixunil:
ACK 9ae3285882
apoelstra:
ACK 9ae3285882
Tree-SHA512: 9d84eb3e2e5fbf80d912193bfa8210a4763388ae246a36cb409416ebbc7e1397b7bc894ba21ec01c416369ecddf1c6d93062c10397f6cff47bd7964f546a8709
ae2aaaa436 Add `script_pubkey_lens` method (Martin Habovstiak)
cf068d16b0 Implement transaction weight prediction (Martin Habovstiak)
Pull request description:
When creating a transaction one must know the the fee beforehand to set
appropriate amounts for outputs and to know the fee, weight is required.
So far we only had a method on an already-constructed transaction. This
method clearly wasn't helpful when constructing the transaction except
for hacks like temporarily adding an all-zeroes signature.
This change adds a function that can compute the transaction weight
without knowing individual bytes of the scripts, witnesses and other
elements. It only needs to know their sizes.
To make the API less error-prone a special, trivial, type is also added
for computing the lengths of witnesses.
Based on #1627
ACKs for top commit:
apoelstra:
ACK ae2aaaa436
tcharding:
ACK ae2aaaa436
Tree-SHA512: 55376601c2c2826bb0909cc25ff5b65816f0b1a2d57fb2cd8831f3db5382de0f4a364d518b312f0528bb5f44c30f3f74f8d254145eed2bfd65e2332b7c4d7c8b
6be89bf94f Add `minimal_non_dust` to `TxOut` (Martin Habovstiak)
Pull request description:
In some scenarios it's useful to create outputs with minimal relayable value. E.g. outputs designated for fee bumping using CPFP. A method for this is useful.
This implements a constructor of `TxOut` that computes the minimal non-dust value from the passed script.
Closes#1459
This one is quite easy, so if we could get it in 0.30, that'd be great.
ACKs for top commit:
apoelstra:
ACK 6be89bf94f
tcharding:
ACK 6be89bf94f
Tree-SHA512: f31ae5f649fbba95ccaabf465cb814df193e7ef89c6e0de7b316a2a484e172beada0da8851da96b195a69a4da1b0991741d4c119f9b0c94fff34150e4f033bd5
We recently created a `types` subdirectory under `script` to keep all
the `Script` and `ScriptBuf` impls together. Turns out this additional
level of subdirectory is a bit annoying and we can achieve the same
grouping by just using `script/mod.rs`.
Move code from `types/mod.rs` to `script/mod.rs`, move the two
submodules up a level, remove the `types` directory.
Currently we have a few things mixed up in the feature gating of
`hashes`.
Observe that:
- `io::Write` is not related to allocation.
- "std" should be able to enable "alloc".
Improve feature gating by doing:
- Enable "alloc" from "std" and simplify `cfg` codebase wide.
- Enable "internals/alloc" from "alloc".
- Fix feature gating to use the minimal requirement i.e., "alloc".
hashmap
Andrew Poelstra
Tobin C. Harding
Martin Habovstiak
Lorenzo Maturano
Harshil Jani