06f1f027ab Make `hash_newtype` evocative of the output (Martin Habovstiak)
b018f3e90b Remove the `$len` argument from `hash_newtype` (Martin Habovstiak)
752817e20d Stop using `$len` in `hash_newtype` (Martin Habovstiak)
Pull request description:
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
ACKs for top commit:
apoelstra:
ACK 06f1f027ab
Tree-SHA512: 9762db1eca9cd749980e5d68ca286f6c926620295a602f62365f199c7b333334b976db25ba25c64e56403cd1ba046b21b99e1c73cc528ad95612ef8901f216e5
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
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
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
"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.
`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.
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
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
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.
In some cases people construct the transaction with a dummy fee output
value before calculating the weight. A method to create the iterator
over `script_pubkey` lengths is useful in such cases.
097e4e9c7f Fix license on bip158 module (Tobin C. Harding)
Pull request description:
When we introduced the SPDX license blurb in [0] we incorrectly gave attribution to Andrew when the original file author had the attribution as "the rust-bitcoin developers". The original author [1] was Tamas Blummer and he copied this code from code he wrote and explicitly re-licenses it. In order to make the re-licensing comment a little clearer and fix the mis-attribution use Tamas' name in the attribution.
[0] commit: `91ff2f628ce7db732d234a812e29fa8508f501a1 Introduce SPDX license identifiers`
[1] commit: `c93a70487f81a93c7d479ae046c75590d9fb7733 Add client side block filter (BIP158) (#281)`
ACKs for top commit:
apoelstra:
ACK 097e4e9c7f
Kixunil:
ACK 097e4e9c7f
Tree-SHA512: cb80d32c739ad562b2d657a34355bb28b1dd5c477b03018fbfbb14de40e03b806663aee89b578bcd8c681b067aa8d02611d4cde36e6fb9a8fa84ad4baf2e290e
6d99d3c061 Use ignore to stop rustdoc code from being built (Tobin C. Harding)
Pull request description:
Currently we have an attempted tag ```compile_fail that seems to be aiming at allowing code that does not build to exist in rustdoc. This is causing an error when running tests.
No clue how this made it through CI.
Use ```ignore to prevent rustdoc code from being built.
ACKs for top commit:
apoelstra:
ACK 6d99d3c061
Kixunil:
ACK 6d99d3c061
Tree-SHA512: 6c4b076000ba29377ac8cf942df66e849ff6421da6f9214664d487550cf45889e163b4de652079010bae327019163b63a1962ff8e6a04d918db63ffb0285ccd1
5f86b3091c Add From<Address> for ScriptBuf (Tobin C. Harding)
Pull request description:
Add an implementation of `From<Address> for ScriptBuf` that calls through to `address.script_pubkey` (which calls
`address.payload.script_pubkey()`).
Fix: #1457
ACKs for top commit:
apoelstra:
ACK 5f86b3091c
Kixunil:
ACK 5f86b3091c
Tree-SHA512: 8a45f292578765b345863946b276607d561b9bc75f6b9bb97f48b32d503143e234aedb658997db802c87289576361ec9ee6cb31fe3bbccfc06cc2fdabc7c41bb
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
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.
41f2dcf6ae Improve test coverage for docs build (Tobin C. Harding)
b4c14a4b7c hashes: Use automatic link (Tobin C. Harding)
96e8a080d1 ci: Remove redundant || exit (Tobin C. Harding)
Pull request description:
Currently the docs build commands in `hashes` and `bitcoin` differ, they should be the same.
Add a command `cargo doc` to improve coverage e.g., recently we botched the feature guarding but since CI only runs `cargo rustdoc` with custom compiler conditional set we didn't catch it.
Done after seeing: https://github.com/rust-bitcoin/rust-bitcoin/pull/1504 and CI should fail on this PR until 1504 is in.
ACKs for top commit:
apoelstra:
ACK 41f2dcf6ae
Kixunil:
ACK 41f2dcf6ae
Tree-SHA512: 7cde68292cfc6f32b75d066e188e7c418ee251f9a5abc57fbd642ba33e9cd5bd8ef7c5ba7cffd206acae6ddec2f8c3db38c8c911a4319e979158666b8225953d
a7117bf8f1 Document source of logic fro read_scriptint (Tobin C. Harding)
2eb2420b40 Add comment on rountripping read/write scripint (Tobin C. Harding)
657dd51e8b Use OP_0 to better mimic bitcoin core code (Tobin C. Harding)
31d254a6a8 Fix push operators URL (Tobin C. Harding)
84cd4ca964 Deprecate script::read_uint (Tobin C. Harding)
Pull request description:
Patch one does the deprecation, the rest of the PR is made up of tiny improvements to the code around reading/writing 'scriptint's (conceptually `CScriptNum`s). I did all this while trying to decipher the discussion on #1547.
### Note Please
There are many more changes in the pipeline for all this read/write "script int" stuff. This PR was done ages ago and I believe it stall adds value.
I re-did the whole PR manually because of the recent `script` module changes. I hope no one else has to do that - if you do please feel free to holla and I'll "rebase" your PR for you.
ACKs for top commit:
Kixunil:
ACK a7117bf8f1
apoelstra:
ACK a7117bf8f1
Tree-SHA512: 5e8ee7fa8d1393a1a50e4241dd947b837cc0ddd15ff1239a49e4839489459fb95d184d6773f73633d55c436310bfab0c73f806d492ed4a4215f924c6c0993936
1e0e712bb0 Add push_* methods for lock times (Tobin C. Harding)
Pull request description:
Lock times are `u32` and can require encoding using 5 bytes.
Add methods `push_lock_time` and `push_sequence` for pushing absolute lock times and sequence numbers. We do not push relative locktimes because they are only 16 bits from the original sequence number.
ACKs for top commit:
Kixunil:
ACK 1e0e712bb0
apoelstra:
ACK 1e0e712bb0
Tree-SHA512: 4b511679270e7ef73937259ccf7d1b9b4b7512b2464f302310519a6e02d55c9cc24e3559302aeb671156e68130478258c1c565f474880e8be708b0ee234e67ff
8596e402f2 Fix docs.rs to use all features (Tobin C. Harding)
89086d094d hashes: Use angle braces to make hyperlinks (Tobin C. Harding)
9b5c2ad7af hashes: Clean up optional dependencies (Tobin C. Harding)
5b4f19c01f hashes: Improve std/alloc features (Tobin C. Harding)
132d2f90b6 bitcoin: Enable alloc feature in features list (Tobin C. Harding)
aa62ca224a hashes: Do not enable core2/alloc feature (Tobin C. Harding)
c15f8dee29 Improve manifest package section (Tobin C. Harding)
12f5e37ed9 Add excludes to manifests (Tobin C. Harding)
Pull request description:
Do a complete overhaul of the manifest of the top level crates (i.e., not `embedded`, `fuzzing` ect.).
Many of the problems being fixed here were introduced over the last year by my poor understanding of exactly what was going on with _every_ line of code in the manifests, after this PR I hope that is no longer a problem.
I'm closing #1571 because it is now done more fully at the end of this PR.
During review please be liberal with any questions so we can ensure everything is spot on now - as we add more crates there are going to be a proliferation of manifest files, best get it right now.
ACKs for top commit:
apoelstra:
ACK 8596e402f2
Kixunil:
ACK 8596e402f2
Tree-SHA512: 9e4ae21221cd5b185c04b6d1af73983d324e6d4e09abf94763f6eabcff420d9a3b2ec62a88cd20844fda518756862b0e64a85be54187166f3b1b2b206b82cb08
861fdd6ab1 Put the `MerkleBlock` struct at the top of the file (Tobin C. Harding)
f0d968197a Put error at the bottom of the file (Tobin C. Harding)
19e094788f Use self for Error variant imports (Tobin C. Harding)
83c2a552db Put helper function below where its called (Tobin C. Harding)
5076579fb9 Fix indentation in pmt_tests macro (Tobin C. Harding)
a7edbfb52e Move hex data to tests/data (Tobin C. Harding)
Pull request description:
PR 2 in the `merkle_tree::block` series, used to be on top of the now merged #1374.
Do a bunch of refactorings in preparation for more invasive changes. This is a separate PR because, other than the first patch which moves hex strings to `tests/data/` the other patches are refactoring only patches, no logic changes. However the last patch is big and will be annoying to review - sorry about that. If you really oppose this basically stylistic patch putting important things first, the opposite of C code, please say and I'll try to stop doing it.
ACKs for top commit:
Kixunil:
ACK 861fdd6ab1
apoelstra:
ACK 861fdd6ab1
Tree-SHA512: 3da0a600898f490b602ab05a396061587d86ffef55697877885a8c611eff96e7382a2d816fe9594c100d378dc56fe7fdc88009a0343bc602b7f4c180836adbd3
Currently we implement string parsing for height/time from the
`absolute` module but not the `relative` module.
Improve the macros used to implement string parsing and use the new
versions to implement string parsing for the height and time types in
`relative`.
Lock times are u32 and can necessitate encoding using 5 bytes. As such
they are "special".
Add methods `push_lock_time` and `push_sequence` for pushing absolute
lock times and sequence numbers. We do not push relative locktimes
because they are only 16 bits from the original sequence number.
Our `script::read_scriptint` function is based on the constructor
code (incl. call to `set_vch`) code from Bitcoin Core. Add rustdoc
comment saying so, emit a link because there are already multiple links
to `script.h` in this file (one just right below the added comment).
We only support reads of upto 4 bytes where as Bitcoin Core allows
reading a `CScriptNum` with more bytes than that. Add a rustdoc
comment (incl. link to Bitcoin Core) mentioning that.
Our `Builder::push_int` method is the same as Bitcoin Core `CScript`
`push_int64` method. We currently use `OP_FALSE` (equivalent to `OP_0`)
but recently we added `OP_0`, lets use it to make our code better mimic
Core (also saves devs checking that `OP_FALSE` is the same as `OP_0`).
The `MerkleBlock` struct is the main type in this file, put it at the
top of the file. This leaves the next most important type,
`PartialMerkleTree` below that.
Refactor only, no logic changes.
70cf4515db Add `Weight` and `FeeRate` newtypes (Martin Habovstiak)
Pull request description:
Use of general-purpose integers is often error-prone and annoying. We're working towards improving it by introducing newtypes.
This adds newtypes for weight and fee rate to make fee computation easier and more readable. Note however that this dosn't change the type for individual parts of the transaction since computing the total weight is not as simple as summing them up and we want to avoid such confusion.
Part of #630
Replaces #1607 (I want to get this in quickly and don't want to be blocked on DanGould's availability.)
ACKs for top commit:
apoelstra:
ACK 70cf4515db
tcharding:
ACK 70cf4515db
Tree-SHA512: ab9cc9f554a52ab0109ff23565b3e2cb2d3f609b557457b4afd8763e3e1b418aecbb3d22733e33304e858ecf900904a1af6e6fdc16dc21483b9ef84f56f103b2
a9108d3939 Refactor script module (Tobin C. Harding)
Pull request description:
The `script` module is large and unwieldy.
Refactor the `script` module, splitting it up into a tree of modules. Here are a few of the changes and their stated benefits
- Split the two script types out into separate files: Readers of the methods can then tell immediately from the file name which type they are reading.
- Put all the impls for the two script types together: Makes parsing the API easier because one can more quickly see which traits are implemented on what i.e., all the `AsRef` imlps are grouped together.
- Put the impls for the two script types in order, first `Script` then `ScriptBuf`: Makes it easier for us to see if we missed something.
- Put the `Builder` and `Instruction` (and associated) types in their own modules: Some devs find long files hard to navigate, so far there hasn't been too much push back against short files.
- Put tests in a separate file: This idea was recently discussed.
This is only moving code and fixing import statements etc. No other changes to the code.
## Note to reviewers
This PR is impossible to review from the diff because it moves so much code. Perhaps better to look at the resulting `src/blockdata/script/` directory and see if you like it.
#### Motivation
While adding script tagging I was having difficulty navigating the script module.
ACKs for top commit:
apoelstra:
ACK a9108d3939
Kixunil:
ACK a9108d3939
Tree-SHA512: 19123c8cfbdce6c42b322fa75a74073a0114b0ed21bd06ca5727981b3573b74cf05075723b774b92ae2b497e20644fca6e2fac14e30cc44f2802dde5aa567f66
Use of general-purpose integers is often error-prone and annoying. We're
working towards improving it by introducing newtypes.
This adds newtypes for weight and fee rate to make fee computation
easier and more readable. Note however that this dosn't change the type
for individual parts of the transaction since computing the total weight
is not as simple as summing them up and we want to avoid such confusion.
Part of #630
In `bitcoin` when we use the `core2` dependency we always need the
"alloc" feature. Enabling "alloc" when enabling "core2" in the "no-std"
feature is confusing because it makes it seem that we don't always need
it.
Set usage of the "alloc" feature of `core2` in the `features` list.
`core2` is for Read/Write, nothing to do with allocation and we do not
use the "alloc" feature of `core2` in `hashes`.
Fix core2 dependency/features by doing:
- Explicitly enable "bitcoin_hashes/core2" in `bitcoin`.
- Do not enable "core2/alloc" in `hashes`
Improve all manifest package sections by doing:
- Order the list of options uniformly
- Remove unnecessary homepage option (currently same as repo)
- Add categories section
We can check which files are included in the packaged release with
`cargo package --list `.
Add an `exclude` section to each manifest that excludes `tests/` and
`contrib/`. Not all crates have a `tests/` directory yet but they should
so add the exclude anyway to future proof the crates.
32d2d62e0f Rename from_slice methods to decode (Tobin C. Harding)
Pull request description:
The `TaprootMerkleBranch` and `ControlBlock` both have methods on them called `from_slice` but these methods do more that just basic copy from a slice. `decode` is a more descriptive name.
Deprecate the `from_slice` methods and implement `decode`, on other changes to the logic.
cc sanket1729
ACKs for top commit:
apoelstra:
ACK 32d2d62e0f
Kixunil:
ACK 32d2d62e0f
Tree-SHA512: e8c089545411a214ef9393f65d3990be46983000bd045182cc27dd70b62273bf48ac97adaf89d1e7fc807c72964a01eef176c7685684e8f87a01c219746d6d3d
55d17f2c18 Instruct devs to use nightly for embedded (Tobin C. Harding)
Pull request description:
The embedded test crate requires usage of the nightly toolchain, fix the docs to show this.
ACKs for top commit:
apoelstra:
ACK 55d17f2c18
Kixunil:
ACK 55d17f2c18
Tree-SHA512: 5d3d611ff4331d8475f77d260ebaa3da3ae71960b709eb4603056d7034885306b8bcc39be488f3391697d47a1074a695979fb50b6cb31af414f873da8e82bdbd
86f372774b Add '_ back into the BitStreamWriter (Tobin C. Harding)
Pull request description:
Recently we merged `commit 53d4fe66b57c255086def2b5e47afaddee776b75` to fix CI even though a better approach is to use `'_` because it assists reading the code (shows that the bit stream writer is not writing from a reference since its writing a `Copy` type `n`).
Add back in the `'_` (I forget what its called).
ACKs for top commit:
apoelstra:
ACK 86f372774b
Kixunil:
ACK 86f372774b
Tree-SHA512: 2a9989164562dbe7bf133e3aeb090fbff7831bfeefb0ac8431e75b17d57184c4d60ac206578c6ebbcff903a3832502a162027ed9f37e5ed87e42a6bf61efa594
The `script` module is large and unwieldy.
Refactor the `script` module, splitting it up into a tree of modules.
Here are a few of the changes and their stated benefits
- Split the two script types out into separate files: Readers of the
methods can then tell immediately from the file name which type they are
reading.
- Put all the impls for the two script types together: Makes parsing the
API easier because one can more quickly see which traits are implemented
on what i.e., all the `AsRef` imlps are grouped together.
- Put the impls for the two script types in order, first `Script` then
`ScriptBuf`: Makes it easier for us to see if we missed something.
- Put the `Builder` and `Instruction` (and associated) types in their
own modules: Some devs find long files hard to navigate, so far there
hasn't been too much push back against short files.
- Put tests in a separate file: This idea was recently discussed.
This is only moving code and fixing import statements etc. No other
changes to the code.
The `TaprootMerkleBranch` and `ControlBlock` both have methods on them
called `from_slice` but these methods do more that just basic copy from
a slice. `decode` is a more descriptive name.
Deprecate the `from_slice` methods and implement `decode`, on other
changes to the logic.
75b266a129 Improve `sighash` module documentation (Martin Habovstiak)
Pull request description:
"Sighash" is a technical term that newbies in Bitcoin may not know and could get lost when trying to find how to sign a transaction. This change attempts to make it more obvious that this module is needed for signing.
Closes#1463
ACKs for top commit:
tcharding:
ACK 75b266a129
apoelstra:
ACK 75b266a129
Tree-SHA512: 7157566c1639c63ce0fba2832e8e5e846e689d89e24077ed7769b721c5db4613cd7fd8d91464992eb78de74b42912ca877e7182a9c3c9c8848bf94d89767b8cc
b3188bbac3 Add `Transaction` accessors to `SighashCache` (Martin Habovstiak)
7c6854fe02 Use `Borrow` instead of `Deref` in `SighashCache` (Martin Habovstiak)
Pull request description:
This changes the bound from `Deref<Target = Transaction>` to `Borrow<Transaction>` (with respective `mut` changes) and adds accessors.
Closes#1423 (PSBT stuff will be separate issue).
ACKs for top commit:
tcharding:
ACK b3188bbac3
apoelstra:
ACK b3188bbac3
Tree-SHA512: 9db2c5890b26e9eefd483d697b42e84b1d7d3b8676fc39b4f39075c149e12697aa538828a757f9187578a958d72a592bb913f8f5788c93feb273db5370979d99
118a593c89 Implement from arrays for TaprootMerkleBranch (Tobin C. Harding)
Pull request description:
The `TaprootMerkleBranch` contains a vector of `TapNodeHash`s, as such it can trivially be constructed from an array of the same type.
Implement `From` for all array sizes 1 - 128 inclusive.
Fix: #1469
ACKs for top commit:
Kixunil:
ACK 118a593c89
apoelstra:
ACK 118a593c89
Tree-SHA512: dd497abd9143ea8b43485133beaccac9049fb915a95a3422d41c1f99961b59ec95df93efe759aa02f62ba1cf3e1afc4597671f1202ff0fa78eeee8b305d21305
The `TaprootMerkleBranch` contains a vector of `TapNodeHash`s, as such
it can trivially be constructed from an array of the same type.
Implement `From` for all array sizes 1 - 128 inclusive.
"Sighash" is a technical term that newbies in Bitcoin may not know and
could get lost when trying to find how to sign a transaction. This
change attempts to make it more obvious that this module is needed for
signing.
Closes#1463
277e8e96bd Add KeyPair import to rustdoc example (Tobin C. Harding)
Pull request description:
Recently, and bizarrely, a PR merged that broke `cargo test --doc`.
Add an import for `KeyPair` to the `schnorr` rustdoc example.
ACKs for top commit:
apoelstra:
ACK 277e8e96bd
Kixunil:
ACK 277e8e96bd
Tree-SHA512: ad214b668827b35848cc7b260cbd2104a916a82a5a6d242bdc498c62edc9a0e864f4bdb4abcade42924dbaf951223ae80feacbe68d8a4ccb4562d8ead50b23a9
bb612fdafa Set rustv_1_53 in build script (Tobin C. Harding)
Pull request description:
The rust version is supposed to be set by the build script so that users automagically get features matching the toolchain in use. Currently we have a feature in the manifest for `rustv_1_53` instead setting a compiler conditional configuration option in the build script. This causes `cargo +1.41.1 --all-features check` to fail.
## Note
I don't see `rustv_1_46` used anywhere, do we need that still?
ACKs for top commit:
apoelstra:
ACK bb612fdafa
Kixunil:
ACK bb612fdafa
Tree-SHA512: f74195d4ee5a5bc5f209e99d30789df3552cef10aee5ea8b61a5a701b753999c34d04be9fe0321ccee7a8ec14fa5a05e0b454b9dc5f8deddd7b5b8d4f3d7e744
It may be useful to access the transaction stored in `SighashCache`
during signing or afterwards, especially when the transaction is stored
without indirection (to enable long-lived storage).
This change adds the appropriate accessors.
The requirement for a type dereferencing to `Transaction` prevented
storing the cache in long-lived data without resorting to silly
wrappers. Since `Borrow` is implemented both for `T` and for smart
pointers it's a more flexible bound which this change implements.
While this is technically breaking, all usual non-generic code will
continue to work beause smart pointers generally have `Borrow`
implemented.
Currently we use a wildcard to export all the hash types in
`hash_types`. We are moving to a world were we only export
normal/standard types from the crate root.
Remove the reexport of the following hash types:
- `FilterHash`
- `FilterHeader`
- `TxMerkleNode`
- `WitnessCommitment`
- `WitnessMerkleNode`
- `XpubIdentifier`
- `Sighash`
Fix: #1541
f0e4e38844 Add newline in rustdoc (Tobin C. Harding)
Pull request description:
Docs created with the `sha256t_hash_newtype` macro are missing a newline between the doc heading and doc main section.
Note that the strings used span multiple lines and therefor the subsequent lines must be aligned with the start of the line (not indented).
Fix: #1540
ACKs for top commit:
Kixunil:
ACK f0e4e38844
apoelstra:
ACK f0e4e38844
Tree-SHA512: 240c68864da63688c400498903d5cc345bee224dcd3235df0127dcf391c66ee08c487d31fe59f890009c674574810b689d9a53628d07d8cdd46b79bc0ac3eb2b
Docs created with the `sha256t_hash_newtype` macro are missing a newline
between the doc heading and doc main section.
Note that the strings used span multiple lines and therefor the
subsequent lines must be aligned with the start of the line (not
indented).
Fix: #1540
The rust version is supposed to be set by the build script so that users
automagically get features matching the toolchain in use. Currently we
have a feature in the manifest for `rustv_1_53` instead setting a
compiler conditional configuration option in the build script. This
causes `cargo +1.41.1 --all-features check` to fail.
32ca6cc320 Remove hex_from_slice and display Sighash forwards (Tobin C. Harding)
a308e1e2ea Remove FromHex for all types except Vec and array (Tobin C. Harding)
3e70c01826 Manually format a bunch of vecs (Tobin C. Harding)
83e1c40c4d Remove script:: prefix from unambiguous types (Tobin C. Harding)
5ab5c264d2 Use fully qualified path in macro (Tobin C. Harding)
7e85452cd9 hashes: Implement std::error::Error (Tobin C. Harding)
5e3abc5e11 Fix feature gating on unit tests (Tobin C. Harding)
3344cf6be2 Favour $reverse instead of $reversed (Tobin C. Harding)
Pull request description:
This work started out, as the branch name suggests, as an effort to use the `hex_lit` crate. But once I got to this stage it seems that the `hex!` macro we have provides different, useful, functionality than the `hex_lit::hex!` macro (it allows usage with non-consts). So I'm unsure if we want to remove it now.
- Patches 1 - 6 are preparatory clean ups
- Patch 7 reduces usage of `FromHex`, please see git log for full description
- Patch 8 removes `hex_from_slice` and fixes a bug in how we display `Sighash`
ACKs for top commit:
apoelstra:
ACK 32ca6cc320
Kixunil:
ACK 32ca6cc320
Tree-SHA512: 11b45b39ec2fc0f837d7395b5fb86de7cc44641fd51cf7e93394a635e6a8fb1c7ac441a6070d5516dae60e084c04cc6e8b605a5167093f964679e445ef60c271
facaefc49c Add conversions for TweakedKeyPair -> TweakedPublicKey (Tobin C. Harding)
2407f241e4 Remove sep256k1 path from Parity (Tobin C. Harding)
Pull request description:
It is trivially possible to get `TweakedPublicKey` from a `TweakedKeyPair`, add conversion methods for doing so.
Patch 1 is preparatory cleanup. Please note `From` is not implemented because the conversion returns the `Parity` also.
Fix: #1452
ACKs for top commit:
apoelstra:
ACK facaefc49c
Kixunil:
ACK facaefc49c
Tree-SHA512: 597026c481fe2622a625cbeb381cac345af6f49f4a115418b69817345fc3c2140bbdbc5208eae1149d7d171f94c776365d302ffe1f9c01d944e738807db28a89
`Sighash` should be displayed forwards according to BIP143. Currently we
are displaying it backwards (as we do for double SHA256). This is
working because parse using `Vec::from_hex`.
We have the means to parse hex strings directly for hashes, we no longer
need `hex_from_slice`.
BIP143 test vectors display double SHA256 forwards but we display
backwards, this is acceptable because there is no fixed display in the
ecosystem for double SHA256 hashes. In order to overcome this we parse
test vector hex strings with into `Vec` when needed.
Remove `FromHex` from hash and script types
- Remove the `FromHex` implementation from hash types and `ScriptBuf`
- Remove the `FromStr` implementation from `ScriptBuf` because it does not
roundtrip with `Display`.
- Implement a method `from_hex` on `ScriptBuf`.
- Implement `FromStr` on hash types using a fixed size array.
This leaves `FromHex` implementations only on `Vec` and fixed size arrays.
In preparation for modifying some unit test data structures, manually
format the code so it is uniform.
Move elements added to a vec with `vec!` onto a new line so they all
line up and one can better see what fields go where.
Refactor only, no logic changes.
Andrew Poelstra
hashmap
Tobin C. Harding
Martin Habovstiak
Lorenzo Maturano
Harshil Jani
DanGould