In preparation for moving the `Witness` to `primitives` we need a way to
construct the `Witness` when decoding. In order to maintain the current
performance and not introduce additional allocations we need to be able
to construct a `Witness` from the content buffer, this leaks the
implementation details of `Witness`.
Add a clearly marked unstable constructor to create a `Witness` from
parts. This introduces the concept of `foo__unstable` function names;
add a section to the README describing unstable functions and semver
guarantees.
88b53a471e Unify deprecated note field format (Jamil Lambert, PhD)
Pull request description:
Following the suggestion in Issue #3306 all the deprecated note fields have been changed to be lower case and in the format "use `abc` instead".
ACKs for top commit:
tcharding:
ACK 88b53a471e
apoelstra:
ACK 88b53a471e successfully ran local tests
Tree-SHA512: 5c20eda7140f37ce78eb58dfdf03ecc11a67fcb10f294d860e81eaaee696e3a4209516017a9885cef9bfff1aa69b845534d139578b674933770fa24d59e4275f
8696cb4b07 Fix unused import warning (Jamil Lambert, PhD)
fbc7aa7fd5 Remove unnecessary lifetimes (Jamil Lambert, PhD)
Pull request description:
New lint warnings from a recent nightly toolchain show some explicit lifetimes that can be omitted.
The unnecessary lifetimes have been removed.
An unused import warning has also been fixed by placing the use statement behind the relevant feature gate.
ACKs for top commit:
tcharding:
ACK 8696cb4b07
apoelstra:
ACK 8696cb4b07 successfully ran local tests
Tree-SHA512: 37e14ca9be2e8cfa64498084a7363783c2cca92dfce40a0a3ed9bda98f081c1fa9119e5d49d81684b6968329945c7004b86534412fd2fcd4898b862c859d42c6
2f656f77ba psbt: Use u64 for key type (Tobin C. Harding)
Pull request description:
Currently we use `u8` for key type but it was pointed out that we should be using a `u64` and encoding it as a compact type. The reason our code works now is because the compact type encoding for a `u8` (less than
This breaks the `serde` impl, as shown by changes to the regression tests,
Fix: #2891
ACKs for top commit:
apoelstra:
ACK 2f656f77ba successfully ran local tests
Tree-SHA512: ce5fe46b54cb724a0b0f9f874c037e5b5d344e5d3c380f9cdb3fdb5cbc5e31e4e32c229f5f2f72547823238934f6b0c3640c2c2ce79d98aa57bac509d130cb82
We use `TBD` in our `deprecated` string and it was discovered that there
is an exception on this string so as not to warn because it is used
internally by the Rust language. However there is a special lint to
enable warnings, lets use it.
Add `#![warn(deprecated_in_future)]` to the coding conventions section
of all crates except `fuzz`.
We had an initial go at this but we didn't do the `Hash` trait method.
In order to do so we need to hack the serde code a fair bit, note the
public visitor types.
Recently we deprecated `to_vec` in favour of `to_bytes` however we
continued to use `to_vec` in a few places. This wasn't noticed because
of our usage of `TBD` in the `deprecated` attribute.
Use `to_bytes` instead of `to_vec`.
d649c06238 Move script types to primitives (Tobin C. Harding)
ec4635904b Inline bytes_to_asm_fmt into Script::Display impl (Tobin C. Harding)
Pull request description:
First patch removes `bytes_to_asm_fmt` as requested by Kix here: https://github.com/rust-bitcoin/rust-bitcoin/pull/3194#discussion_r1756557768
Second patch does the move. The move is minimal but there is quite a bit of code moved in `script/mod.rs` - I believe it is as minimal as required as well.
ACKs for top commit:
apoelstra:
ACK d649c06238 successfully ran local tests
Tree-SHA512: 329a23948ac5617402a724b734d81cde8ab1f57ddd4860f858880618e260ea8b5cc89315de1fd93ae32787d5e8508fd604a41f003b1f5772a773b5b1648d382c
c41a6e9b1b feat: add sign fn for sign_message (ChrisCho-H)
Pull request description:
While it's not hard to create the signature using `secp256k1` modules with `signed_msg_hash`, it's much more convenient and safe to provide one-way function to generate signed signature(even without the understanding about the semantics of bitcoin message signing).
ACKs for top commit:
apoelstra:
ACK c41a6e9b1b successfully ran local tests
tcharding:
ACK c41a6e9b1b
Tree-SHA512: 84caea275059381040c71100badb54556dd105722f79ac43d2df7eb0e5428cf8e7acc2d7f262625dd008837099a928c3c8be858f9ab11838c2eec1786e9f1844
9e6b8faf84 feat: add version three variant to transaction version (Rob N)
Pull request description:
Topologically restricted transactions are now considered standard as of Bitcoin 28.0.
ACKs for top commit:
apoelstra:
ACK 9e6b8faf84 successfully ran local tests
tcharding:
ACK 9e6b8faf84
Tree-SHA512: 289b986304e206802f04cee6607087167f6d91d8a81d4fc49ed01c430f4f6ad00b44646fbefdd000148fc5bfe2d257f92b386bfaf4405c482e4e438d830ab586
a250c8eee4 Fix unused imports (Shing Him Ng)
Pull request description:
Found some unused imports while working on something unrelated
ACKs for top commit:
apoelstra:
ACK a250c8eee4 successfully ran local tests
tcharding:
ACK a250c8eee4
Tree-SHA512: ffa4fcddb91c849df7885a6d734405ab704e0353293fa3462a981e179ea49091990df4482eee2324e7b5ef250a0890b2e8983d53ec288ca6e8ada77e30c102dc
0c824c9c68 Implement Arbitrary for Block (Shing Him Ng)
Pull request description:
Implementing `Arbitrary` for `Block` and its child types
ACKs for top commit:
apoelstra:
ACK 0c824c9c68 successfully ran local tests
tcharding:
ACK 0c824c9c68
Tree-SHA512: 407acd4155ca7496bf7c59af19f103b22b4c74dd013c2e2c42aae690ba7264ecd42ed034355cf8895f2532f8cef77dfdbeac250301a6973afd5f3920be7e4310
323e706113 Add rustfmt config option style_edition (Tobin C. Harding)
2e4179ed0f Run the formatter (Tobin C. Harding)
2c40b4f4ec Configure formmater to skip read_compact_size (Tobin C. Harding)
Pull request description:
`rustfmt` is emitting:
Warning: the `version` option is deprecated. Use `style_edition` instead.
As suggested add a config option and set it to 2021.
- Patch 1: Manually configure rustfmt to skip some code
- Patch 2: Run the formmater with current configuration
- Patch 3: Add the new config option (remove old one), introduces no new formatting requirements
ACKs for top commit:
apoelstra:
ACK 323e706113 successfully ran local tests
Tree-SHA512: 7f80cc89f86d2d50936e51704344955fa00532424c29c0ee3fae1a6836e24030f909b770d28da13e1c5efde3d49ad7d52c6d909d120fb09c33abf1755f62cd38
3b7ba4f977 Remove the SliceIndex implementation from hash types (Tobin C. Harding)
Pull request description:
If folk really want to index into a hash they can us `as_byte_array` then index that.
Includes a bump to the version number of `hashes` to `v0.15.0` - this is because otherwise `secp` won't build since we are breaking an API that is used in the current release of secp.
Fix: #3115
ACKs for top commit:
apoelstra:
ACK 3b7ba4f977 successfully ran local tests
Tree-SHA512: 0ba93268cd8133fe683183c5e39ab8b3bf25c15bfa5767d2934d67a5f6a0d2f65f6c9304952315fe8a33abfce488d810a8d28400a28facfb658879ed06acca63
70264bfcec Move block::Version to primitives (Tobin C. Harding)
819d8d72e8 Stop using private Version constructor (Tobin C. Harding)
Pull request description:
This is a straight up move of the whole type because there are only three methods, a getter, a setter, and `is_signalling_soft_fork`.
If we use an extension trait for `is_signalling_soft_fork` then we have to make the two private associated consts public which is in my opinion worse.
Patch 1 is preparation, use getter/setter, patch 2 does the move.
ACKs for top commit:
apoelstra:
ACK 70264bfcec successfully ran local tests
Tree-SHA512: b61e853b4b96cb1cc56c7bfb67cc6c3ba7c631cb9e540393eb780dcf63bd2d934058794f2ac0145b4b5404a6a68887c3a1d225b2d87b57415474c05d3ef2811f
Currently we use `u8` for key type but it was pointed out that we should
be using a `u64` and encoding it as a compact type. The reason our code
works now is because the compact type encoding for a `u8` (less than
253) is the same as for a `u8`.
This breaks the `serde` impl, as shown by changes to the regression tests.
aa1fd5f44c Update the doc for InputWeightPrediction::weight() (spacebear)
8fd53c8ecf Add nested P2WPKH helpers to InputWeightPrediction (spacebear)
Pull request description:
Following up on https://github.com/rust-bitcoin/rust-bitcoin/issues/630#issuecomment-2392050517.
I amended the docstring on `InputWeightPrediction::weight()` to clarify that it only returns the signature weight. I'm not sure what the rationale was for only returning partial input weight, and if there are any arguments against something like `InputWeightPrediction::total_weight()` which returns `self.weight() + Weight::from_non_witness_data_size(32 + 4 + 4)` (and maybe deprecate `weight()` eventually to prevent misuse).
ACKs for top commit:
apoelstra:
ACK aa1fd5f44c successfully ran local tests
tcharding:
ACK aa1fd5f44c
Tree-SHA512: 9bdd882164a80a1a1a44c593750aa59e470f2928c0380d4d53a0409add597844cf11945785264ebd6b281029b3f2ef1be6c77e1e9cd148192dbd601df30a60ae
If folk really want to index into a hash they can us `as_byte_array`
then index that.
Includes a bump to the version number of `hashes` to `v0.15.0` - this
is because otherwise `secp` won't build since we are breaking an API
that is used in the current release of secp.
Fix: #3115
This is a straight up move of the whole type because there are only
three methods, a getter, a setter, and `is_signalling_soft_fork`.
If we use an extension trait for `is_signalling_soft_fork` then we
have to make the two private associated consts public which is in my
opinion worse.
In preparation for moving the `block::Version` type over to `primitives`
stop using the private constructor and inner field. Use the public
getter/setter instead (`to_consensus`and `from_consensus` respectively).
3f3f30d6c7 Use iter instead of accessing content field (Tobin C. Harding)
6389d1cbb3 Stop using push_slice (Tobin C. Harding)
be163eec99 Use Witness::len instead of accessing field (Tobin C. Harding)
Pull request description:
Prepare `Witness` to be moved to `primitives`.
This is the first three patches out of #3406. Patch 1 and 2 are internal changes, path 3 is also internal but introduces a slight perf hit by doing multiple writes.
ACKs for top commit:
apoelstra:
ACK 3f3f30d6c7 successfully ran local tests
Tree-SHA512: 25e2570a22797dbfa15d6e5af9b72938243192ca264bc5fe82c2f327486e52a73993b3b61ee1161e5d17b01a7f9843960cb4031e6550561de4ecafb9f935e375
We would like to move the `Witness` to `primitives` however in the
`Encodable` implementation we are currently accessing the private
`content` field.
Instead of accessing `content` we can iterate over the witness elements
and write each individually, this has the same result but does a bunch
of additional calls to `Write::write_all` (via `emit_slice`).
This patch effects performance negatively but makes no changes to the
encoding.
The `Witness::push_slice` function is called by `Witness::push` after
calling `as_ref`, hence is equivalent for all types that implement
`AsRef<[u8]>`. Also, `push_slice` is a private method on `Witness`.
In preparation for moving `Witness` over to `primitives` stop using
`push_slice` in favour of `push`.
Internal change only.
In preparation for moving the `Witness` oven to `primitives` use the
`len` function instead of accessing the `witness_elements` field.
No logic change, `Witness::len()` returns `witness_elements`.
da0795e590 primitives: Use doc links for OutPoint (Tobin C. Harding)
b079cbafee Move OutPoint to primitives (Tobin C. Harding)
f5c46cd411 Introduce OutPoint extension traits (Tobin C. Harding)
7e5bd5048d Remove docs on deprecated is_null function (Tobin C. Harding)
97b20a2316 Add additional impl block to OutPoint (Tobin C. Harding)
Pull request description:
Just the minimal move of `OutPoint` to `primitives`.
The last patch closes#3347
ACKs for top commit:
apoelstra:
ACK da0795e590 successfully ran local tests
Tree-SHA512: dced5a6d6bc6af0ce8b4ae4e52c04b45c85eb77a24bb25762ba8ab7deeab1e6b392cc4b258bb14218e8a2af999715dfed45ba94599cb16963a843995a7475917
5fab6b178f Rename iter len unit test (Tobin C. Harding)
f6a74ef4af Refactor the serde Witness unit tests (Tobin C. Harding)
9860453b5b Improve Witness consensus encode unit test (Tobin C. Harding)
7e2899d310 Improve Witness::push unit test (Tobin C. Harding)
fe967279e5 Improve witness unit tests for single empty element (Tobin C. Harding)
Pull request description:
In preparation for moving the `Witness` type over to `primitives` refactor and improve all the unit tests that will be moved, do not touch the ones that will stay behind.
The first five patches are from #3406, the last is just a re-name of the test function I tried to refactor in ac6fe3a881
ACKs for top commit:
apoelstra:
ACK 5fab6b178f successfully ran local tests
Tree-SHA512: bc00f81e3c5cc92ae58dd2fc876d368a487ae6c08cc0735d7227c3a89287e321dbfb5b571b951d0616af0ec7cf9a0ea2d0e724645b1c419933a212ece80a0fbf
2d8c613340 Move the block hash types to primitives (Tobin C. Harding)
6b9429ac7b Remove BlockHash::all_zeros (Tobin C. Harding)
20d8dbd586 Add missing line of whitespace (Tobin C. Harding)
Pull request description:
As an initial step in moving the `block` module, just move over the hash types `BlockHash` and `WitnessCommitment`.
Patch 2 introduces an associated const `BlockHash::GENESIS_PREV_BLOCKHASH` and removes `all_zeros`.
ACKs for top commit:
apoelstra:
ACK 2d8c613340 successfully ran local tests
Tree-SHA512: 64aa0ae81e1c8ab1b5d4cd8cd28e6ef04ed01bf79175dc5b1fd607a6f0967e06b0aaf4c10ad368e2b327edcad3705187b6643d5ca8647716319424f19a838ba1
9ded58fc99 Move merkle_tree hash types to primitives (Tobin C. Harding)
Pull request description:
In preparation for moving the `block::Header` struct over to `primitives` move the `merkle_tree` hash types.
ACKs for top commit:
apoelstra:
ACK 9ded58fc99 successfully ran local tests
Tree-SHA512: 98017cf0403871f01a6efeda22e8f319cc8104b9bc2f3a9bae2d6a31f6df8172307466c6486a9259204166933137fa03e565e08a0c156c278cfeb72cdae09b89
18d8b0e469 Replace VarInt type with ReadExt and WriteExt functions (Steven Roose)
003db025c1 Return encoded length from WriteExt::emit_slice (Steven Roose)
Pull request description:
This the meat and potatoes out of Steven's work in #2133 and also closes#1016
ACKs for top commit:
apoelstra:
ACK 18d8b0e469 successfully ran local tests
Tree-SHA512: 2df96c91e0fbfdc87158bde9bbdd9565f67e3f66601697d0e22341416c0cd45dd69d09637393993f350354a44031bead99fd0d2f006b4fc6e7613aedc4b0a832
e215a39dba Improve documentation of Xpub::from_xpriv (Jiri Jakes)
0dcba98382 Add Xpriv::to_xpub (Jiri Jakes)
5a9341bfc5 Improve naming of methods on Xpub and Xpriv (Jiri Jakes)
Pull request description:
Adds `Xpriv::to_xpub` and makes naming of methods related to extended and non-extended keys on `Xpub` and `Xpriv` consistent and easier to discover:
- if method takes or returns extended key, its name uses `xpriv` or `xpub`
- if method takes or returns non-extended key, its name uses `public_key` or `private_key`
Previous naming of the methods was confusing and unclear and this PR deprecates them.
Closes#3327.
### Notes for reviewers
- `xpriv` and `xpub` could be without `x`, I opted for this version to remove any ambiguity
- also considered `raw` or similar name (as suggested) for methods with non-extended keys, however `raw` is usually not used in this context and `to_public_key` vs. `to_x_only_public_key` did not look alright with any other name
- if splitting this PR into two would be desirable, please let me know
- is there a reason why `ckd_priv` is private and `ckd_pub` is public?
ACKs for top commit:
apoelstra:
ACK e215a39dba successfully ran local tests
tcharding:
ACK e215a39dba
Tree-SHA512: f76b22740b06df80dd9d01fbb991149243e47619e0bd5f1699b446456259cc6e97ecb9fca7b4881e921a9d7341ca92c6ee2dae90a417f702aff5eb760439ca42