2157e69857 Document the `all` module (Tobin C. Harding)
Pull request description:
Improve documentation on the `all` module by doing:
- Document guarantee that `all` will only ever contain opcode constants
- Fix stale/incorrect code comment
Done as follow up to #1295
ACKs for top commit:
apoelstra:
ACK 2157e69857
Kixunil:
ACK 2157e69857
Tree-SHA512: 4df091bbdce7b9ba73caabd74b80f9e8c0a30fa2f9a20ed9b75542e71a204e5cd82698a74bebbd6f0beab55ecd807154d1b7d27a787cc9dede7abbd20a0a4ad5
8c4fa5ec3d Remove into_bytes from impl_array_newtype (Tobin C. Harding)
Pull request description:
All the types that we define with `impl_array_newtype` are `Copy` so the correct conversion method to get the underlying byte array is `to_bytes`. We currently provide `into_bytes` as well as `to_bytes`, with one of them calling `clone` - this is unnecessary and against convention.
Remove `into_bytes` and for `to_bytes` just return the inner field. Add code comment to remind devs of correct usage of the macro in regards to deriving `Copy` and `Clone`.
ACKs for top commit:
Kixunil:
ACK 8c4fa5ec3d
apoelstra:
ACK 8c4fa5ec3d
Tree-SHA512: 619acbccc5d86afeaea9f69e4b8d4290dcd8d2ccb09d5bb22f475a47f703d63a45c059b0261b1cc3d4c8d9ef0a508263e1e836fd3ba6fa08ff67ebb23eb63309
The `all` module enables usage of a wildcard import statement without
muddying the scope with any other types defined in `opcodes`, in other
words if one wants to use the `All` type `opcodes::All` is the most
clear way to use it, however usage of naked `OP_FOO` types is perfectly
clear.
Add documentation stating that we guarantee to never put anything else
in the `all` module so folks are confident using a wildcard import will
not bring any rubbish into scope.
Expected usage in downstream applications that need types in `opcodes`
as well as the opcodes:
```
use bitcoin::opcodes::all::*;
use bitcoin::opcodes;
```
Also, we do no implement `Ord` or `PartialOrd`, document this including
HTML tags hiding an example bug from Bitcoin Core that shows why not.
9a1623c1dc Re-export hashbrown when enabled (Tobin C. Harding)
Pull request description:
`hashbrown` used to be exported until commit 23ee0930c7 which removed the `pub extern crate` declaration.
Found thanks to afilini (#1342)!
ACKs for top commit:
apoelstra:
ACK 9a1623c1dc
Kixunil:
ACK 9a1623c1dc
Tree-SHA512: 0363781dc06211eec59246ce54582220674d5ab2bd6e62ad15eeb97b0df6435cdf23df8306131c176b6003dde31d1e376f52981f2e69e9d2529876d3ada727e0
b6f9e47dba Fix `no_std` when `bitcoinconsensus` is enabled (Martin Habovstiak)
Pull request description:
`default-features = false` was missing previously but blindly adding it would lead to subtle risk of breaking when a crate not needing `std` depends on `bitcoinconsensus` and simultaneously another crate not needing `bitcoinconsensus` depends on `std` and another crate depends on them both.
This change fixes it by introducing `bitcoinconsensus-std` feature flag and provides a fallback if the flag is off. Unfortunately the fallback has to use a bit of reasonable `unsafe` due to limitations of upcasting.
The only safe alternatives are not do it and provide worse experience for crates that are affected by the problem above or break the API, which couldn't be backported and would be more annoying to use.
Closes#1343
This is considered PoC PR as I realized the possibility of the hack (and necessity of `unsafe`) at the last moment. Things like tests and modifying CONTRIBUTING to change the stance on `unsafe` will be added if `unsafe` is ACKed.
ACKs for top commit:
tcharding:
tACK b6f9e47dba
apoelstra:
ACK b6f9e47dba
Tree-SHA512: 3a2845f4701c94ff6214749fa490aecf3fd96089df31b15f9d3e0afe3c74329ff2b9054d51244358a79f928aa9d4cf4001fc3ec40a9b0e189323544c4480c709
b84e1d46c0 Move amount module out of util (Tobin C. Harding)
Pull request description:
Done as part of flattening the `util` module. Simply move the `amount` module out of the `util` module and to the crate root. Justified by the fact that the `Amount` type is more-or-less a "primitive" bitcoin type.
ACKs for top commit:
apoelstra:
ACK b84e1d46c0
Kixunil:
ACK b84e1d46c0
sanket1729:
ACK b84e1d46c0
Tree-SHA512: 9ec707f49b7ab29f573be22b366d2ea9c1a8e4b27e80350d521b9c6607fca4142f079648cb739ba8590edd97c21a00029c3647c4c8cebe47cc2dfee1b10b8b39
All the types that we define with `impl_array_newtype` are
`Copy` so the correct conversion method to get the underlying byte array
is `to_bytes`. We currently provide `into_bytes` as well as `to_bytes`,
with one of them calling `clone` - this is unnecessary and against
convention.
- Remove `into_bytes` and for `to_bytes` just return the inner field.
- Add a method that causes build to fail if `Copy` is not implemented.
29df410ea3 Document state after call to calculate_root_inline (Tobin C. Harding)
2dbc7fdf21 Rename merkle_root functions (Tobin C. Harding)
22dd904735 Rename util::hash module (Tobin C. Harding)
Pull request description:
Done as part of flattening `util`.
The `util::hash` module only provides two functions, both to calculate the merkle root of a list of hashes.
1. Rename `util::hash` -> `crate::merkle_root`
2. Change function names to `calculate[_inline]` so usage becomes `merkle_root::calculate`
Done as two separate patches so we can bikeshed the names, can squash if needed.
ACKs for top commit:
Kixunil:
ACK 29df410ea3
apoelstra:
ACK 29df410ea3
Tree-SHA512: 17ace90c7700b5d7adf8b95731c9a348b5c92863806cc88bc40730547f457e44160efb19985e025970b59fea86d68f0bf4be0af17717a65ae44f11c8d10ec4c6
1050fe9cae Remove unnecessary borrow (Tobin C. Harding)
3966709336 Use is_none() (Tobin C. Harding)
d192052519 Remove unnecessary dereference (Tobin C. Harding)
624cda07b3 Remove unnecessary casts (Tobin C. Harding)
Pull request description:
Clippy has been updated and new warnings are being triggered in our codebase. This PR does all warnings using nightly since they all looked like reasonable things to fix.
Needed for CI to pass in other open PRs.
ACKs for top commit:
Kixunil:
ACK 1050fe9cae
sanket1729:
ACK 1050fe9cae.
Tree-SHA512: 7dcfb6a72a0aae51b49b417bb94cbe1becb1095d1bf0011921b1834a10f792cfcdeee37993ab9b103bd2dfcc9cd3c26cd7f1bb80b06b0d1aa4aaa454bfb0b3f0
d6ca7e4b9f Add a test parsing transaction with a huge witness (Martin Habovstiak)
Pull request description:
This transaction broke past versions of `rust-bitcoin` and LND so this adds a test to avoid reintroducing the problem in the future.
See also https://github.com/romanz/electrs/issues/783
I'm publishing this immediately for research purposes. I can clean it up later if required (low on time rn) or we may even not merge it if there is a better test.
ACKs for top commit:
tcharding:
ACK d6ca7e4b9f
Tree-SHA512: cfa9f5c82be0885a82bddb0c15f3177e05feedb369a931e58bf48d90b26eab85bc501d84d17927fa72b42ad4f1016e6042ad318662403271c738eaa91cee7748
cc0b1f1804 Add a CHANGELOG entry for 0.28.2 (Matt Corallo)
Pull request description:
I'm not actually sure if we want this - we don't seem to (consistently) add entries for point releases, but figured I'd put it up in case we want it.
ACKs for top commit:
tcharding:
ACK cc0b1f1804
Kixunil:
ACK cc0b1f1804
sanket1729:
ACK cc0b1f1804
Tree-SHA512: b41f647cfea925a5acd8eaa8568692e7393dabbfd6c75e9dad9e18f34847353363cadaad9109b41a7b1e5851030fefc0180e6b824a2eaaf8f58b4d62fd4bc68f
This transaction broke past versions of `rust-bitcoin` and LND so this
adds a test to avoid reintroducing the problem in the future.
See also https://github.com/romanz/electrs/issues/783
The function call `calculate_root_inline` calculates the merkle root
using the input array as a scratch buffer, i.e., we trash the data
during recursive calls to `merkle_root_r`.
Add explicit documentation to the function so its super clear not to use
the hashes again after calling this function.
Recently we renamed the `hash` module to `merkle_root`, this makes the
public functions provided stutter if used with one layer of path as is
Rust convention:
`merkle_root::bitcoin_merkle_root`
We can improve on this by renaming the functions to 'calculate', then we
get
- `merkle_root::calculate()`
- `merkle_root::calculate_inline()`
The `util::hash` module provides two functions for computing a merkle
root from a list/iterator of hashes.
Rename the module to `merkle_root` and move it to the crate root,
deprecate the original functions.
Done as part of flattening the `util` module.
d2ed0fe022 Add `impl IntoIterator for &'_ Witness` (Martin Habovstiak)
Pull request description:
It is considered idiomatic for types that have `iter()` method to also implement `IntoIterator` for their references. `Witness` was missing this so it is added here.
ACKs for top commit:
apoelstra:
ACK d2ed0fe022
tcharding:
ACK d2ed0fe022
Tree-SHA512: fc891109696de4f349324d6ddc160249ef22510622d9ce72a65b18f085d86b0de0f3ecb4f7060e1eaf716a908029865cd21cda5a6598fc4c16d0540152d9a4c9
2674327c93 Remove the endian module (Tobin C. Harding)
Pull request description:
Now we have MSRV of 1.41.1 we can use the `from_le_bytes` and `to_be_bytes` methods implemented on standard integer types, these became available in Rust 1.32.
Remove the `endian` module replacing its logic with calls to methods on the respective stdlib integer types.
ACKs for top commit:
Kixunil:
ACK 2674327c93
apoelstra:
ACK 2674327c93
Tree-SHA512: 7cdaf278c9d162cb0080bb6b9ea80ab55f881bfcd389f8b968f8cfaeebb0d27d3b5b46e9677a376bc6b7d4068cf094f50560ed4ae7bc817c50da688f70a7af25
It is considered idiomatic for types that have `iter()` method to also
implement `IntoIterator` for their references. `Witness` was missing
this so it is added here.
b8bd31d5a8 Promote rust-miniscript finalizer (DanGould)
16bf6f68dd Test PSBT integration (DanGould)
6b6ef528a4 Add OP_0 alias for OP_PUSHBYTES_0 (DanGould)
72935a0f6e Move test_data/* tests/data (Tobin Harding)
Pull request description:
resolves#892
* Initial patch adds OP_0 alias for OP_PUSHBYTES_0 as [approved here](https://github.com/rust-bitcoin/rust-bitcoin/pull/935#discussion_r854539416)
* Second patch is the bulk of this work. It tests BIP 174 PSBT integration defined at [The BIP's Test Vectors Section](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#test-vectors) using #957
* Third patch points users to rust-miniscript for a more useful finalizer
ACKs for top commit:
tcharding:
ACK b8bd31d5a8
sanket1729:
ACK b8bd31d5a8. Thanks for sticking with this PR. This looks awesome after #957
Tree-SHA512: dc68e524d4349530b082baf5032460fa56593b0ef192125c0b7d7e00954e5593f386b7f1984fc00106b4b9eafbf29cc80ab368dbd26b710eba0962dbd89e0013
Now we have MSRV of 1.41.1 we can use the `from_le_bytes` and
`to_be_bytes` methods, these became available in Rust 1.32.
Remove the `endian` module replacing its logic with calls to methods on
the respective stdlib integer types.
c4084b91fb Fix broken build due to conflict between #1340 and #999 (Andrew Poelstra)
Pull request description:
I will test merge commits more thoroughly before signing off on them in future, sorry.
ACKs for top commit:
DanGould:
tACK c4084b91fb
tcharding:
ACK c4084b91fb
sanket1729:
ACK c4084b91fb
Tree-SHA512: 51ece3aa43045e81138d21b8402b1ec1559a0b37bdfc4c5246ff46fd085364517449a2e20e625934cbc0c96f18eb2fc6121a6e993fd5b9535ae54c863d032a0b
dea9b1d1e0 Re-export base64 when enabled (Alekos Filini)
Pull request description:
`base64` used to be exported until commit 23ee0930c7 which removed the `pub extern crate` declaration.
ACKs for top commit:
Kixunil:
ACK dea9b1d1e0
apoelstra:
ACK dea9b1d1e0
Tree-SHA512: 2f32b6676aab9881bab9eb0ae61910ec0d4b60cb17c8a7bf8155ec4a13e50abce0061b52f4e81b106b938e99cb68329d027291c1702213cfa2a46734ebadb488
1a89d5230c examples: Add taproot-psbt workflow example (Duncan Dean)
Pull request description:
Will address #893.
Currently includes a BIP86 example (no spendable script path)
Working on script path and key path spending when both are possible spending paths.
ACKs for top commit:
tcharding:
ACK 1a89d5230c
apoelstra:
ACK 1a89d5230c
Tree-SHA512: 31d23914faedb2632d517f9a827075f1bf387df6d98f151000f70546d1e67ac332e698c6a01bda40a9baf5b25bff0114928edc0879c5e01753ecfc6ad182fe26
`default-features = false` was missing previously but blindly adding it
would lead to subtle risk of breaking when a crate not needing `std`
depends on `bitcoinconsensus` and simultaneously another crate not
needing `bitcoinconsensus` depends on `std` and another crate depends on
them both.
This change fixes it by introducing `bitcoinconsensus-std` feature flag
and provides a fallback if the flag is off. Unfortunately the fallback
has to use a bit of reasonable `unsafe` due to limitations of upcasting.
The only safe alternatives are not do it and provide worse experience
for crates that are affected by the problem above or break the API,
which couldn't be backported and would be more annoying to use.
Closes#1343
fd7f8daeff Move sighash module to crate root (Tobin C. Harding)
Pull request description:
Done as part of the effort to flatten the `util` module.
The `sighash` module can stand alone in the crate root, it provides a discreet set of functionality - the `SighashCache` and associated types.
Marking as high priority because this is part of flattening `util` which is a required step before we start crate smashing.
ACKs for top commit:
Kixunil:
ACK fd7f8daeff
apoelstra:
ACK fd7f8daeff
Tree-SHA512: e812ca903f7dccfa5a06084e23f93f617d016583bdf082d7a36ca8e67e49f1d140a3e138b93939e816861460ff2c04d49d5e37a555dd853dca1c76dbccd910bf
b05ba16a05 ci: Remove serde version pinning (Tobin C. Harding)
Pull request description:
The MSRV break in serde is fixed now, remove the serde version pinning.
Fix: #1256
ACKs for top commit:
Kixunil:
ACK b05ba16a05 if CI passes.
apoelstra:
ACK b05ba16a05
Tree-SHA512: 2046b443500a848cb7b22576473f3b09ece4538dd3d30d7a06b09b28d6fe26f5c4d482f70ec0ec1b79469c221dc9c09e8b05b83debb71fd27f7dc30571cbbcfe
c34d5f8f85 Implement PartiallySignedTransaction::fee (hashmap)
Pull request description:
to calculate fee if previous outputs are available.
Closes https://github.com/rust-bitcoin/rust-bitcoin/issues/1220
ACKs for top commit:
Kixunil:
ACK c34d5f8f85 if CI passes
tcharding:
ACK c34d5f8f85
apoelstra:
ACK c34d5f8f85
Tree-SHA512: 697b837de2fb21bbd5d489c524c06a56bb35b73c0f32cc5b0500f5508f3c539b21d327cd556a04ee847ccf8d98829da994d90c19e80c457ddba2cd9d3469476e
Done as part of the effort to flatten the `util` module.
The `sighash` module can stand alone in the crate root, it provides a
discreet set of functionality - the `SighashCache` and associated types.
c3e4399519 Remove usage of opcodes::all (Tobin C. Harding)
Pull request description:
We have all of the opcodes defined in a submodule called `all`, this allows wildcard imports without bringing in the other types in the `opcodes` module.
Use wildcard import `use crate::blockdata::opcodes::all::*` instead of fully qualifying the path to opcodes.
### Original PR description (left here so the thread of discussion below makes sense)
The `all` module adds no value, we can remove it without loss of meaning or clarity, doing so makes the code less verbose.
EDIT: After review, now includes importing with wildcard and removing the `opcodes::` path from any type starting with `OP_`.
Idea stolen from: https://github.com/rust-bitcoin/rust-bitcoin/pull/525 (patch 7)
ACKs for top commit:
Kixunil:
ACK c3e4399519
apoelstra:
ACK c3e4399519
Tree-SHA512: 300511d909a25e82c563b18da1b52bcf65653cd3efd8ff32dd5b9e839dacd57924953c1745dfb5e9301fa4f9fc0cd61a075f3a3fd94f6a5a9730bca5155dfd96
We have all of the opcodes defined in a submodule called `all`, this
allows wildcard imports without bringing in the other types in the
`opcodes` module.
Use wildcard import `use crate::blockdata::opcodes::all::*` instead of
fully qualifying the path to opcodes.
7e39082eec Improve doc of `Script::push_verify` (Martin Habovštiak)
Pull request description:
This rewords the doc to have a reasonable summary, adds a little background explaining the opcode behavior and the effect of the function when called multiple times.
Closes#1154
ACKs for top commit:
tcharding:
ACK 7e39082eec
apoelstra:
ACK 7e39082eec
Tree-SHA512: 7f0142c9fcec8ef5b30779f1d22922219180aa103ce2f3039412b1d6b46aa7ee2522181e23a76f9ba5fd84720ef3ff3daa8233d71cf10008f5e3b805b5a5c470
7d851b42ee Move serde_string_* macros to the serde_utils module (Tobin C. Harding)
53b681b838 Move const_assert to bitcoin_internals (Tobin C. Harding)
5a8a5ff6c9 Move debug_from_display to bitcoin_internals (Tobin C. Harding)
a2f08f2bc6 Improve docs on impl_array_newtype macro (Tobin C. Harding)
771cdde282 Move impl_array_newtype to bitcoin_internals (Tobin C. Harding)
Pull request description:
Move macros out of `internal_macros`, done in an effort to work towards removing the `internal_macros` module since we have `bitcoin_internals` now.
ACKs for top commit:
apoelstra:
ACK 7d851b42ee
Kixunil:
ACK 7d851b42ee
Tree-SHA512: b31b3a5b4d18a2dbe3f358bff62ae6ca4041d432c755e9c45b0241d48903e02c95e79ec72a7478b9d2a53486ce9eef19bfe3b8905aba19036e59c0719f193ce7
This rewords the doc to have a reasonable summary, adds a little background explaining the opcode behavior and the effect of the function when called multiple times.
Closes ##1154
Done as part of flattening the `util` module. Simply move the `amount`
module out of the `util` module and to the crate root. Justified by the
fact that the `Amount` type is more-or-less a "primitive" bitcoin type.
dd8730e14f Use new PSBT signing API in example (Tobin C. Harding)
d2367fb187 Add PSBT sign functionality (Tobin C. Harding)
b80e5aeaab Re-order import statements (Tobin C. Harding)
Pull request description:
Add an API for signing inputs to the `PSBT` struct. This is work based on code in `rust-miniscript` and the API design suggestions below from @sanket1729 and @Kixunil.
Please note, this adds an `unimplemented!` call for taproot inputs. ECDSA signing is complete.
Includes a patch adding the psbt example from https://github.com/rust-bitcoin/rust-bitcoin/pull/940 updated to use this new api. Run `cargo run --example psbt --features=bitcoinconsensus` to test it out.
ACKs for top commit:
dunxen:
ACK dd8730e
apoelstra:
ACK dd8730e14f
sanket1729:
reACK dd8730e14f
Tree-SHA512: 6345571e53cd3aa4b7ad962536da47ae03ab7c0b088107dc4104676bdb64fcf892e8fa60e0b716f3ef158d88d7058938bf267046721ccf74b2d1b092e9b9aaaa
Andrew Poelstra
Tobin C. Harding
sanket1729
Matt Corallo
Martin Habovstiak
DanGould
Alekos Filini
hashmap