On the way re-design the API by doing:
- Introduce `Checked` and `Unchecked` tags
- Rename the `txdata` field to `transactions`
- Make the `Block` fields private
- Add getters for `header` and `transactions` fields
- Move the various `compute_` methods to be free standing functions
- Make the `check_` functions private
- Introduce extension traits
be553217f1 transaction: Add plural field getters (Tobin C. Harding)
Pull request description:
The `Transaction` type was created way back when, and the field names were named using singular (`input` and `output`). In hindsite plural probably should have been used since its more idiomatic Rust but the fields have been around so long now that re-naming them is going annoy a whole bunch of people.
As a compromise add getters that use plural, immutable and mutable versions, for both the `input` and `output` fields.
Close: #822
ACKs for top commit:
apoelstra:
ACK be553217f19069dd9ca28c3ba5953059df9caa1b; successfully ran local tests; though will let this sit a couple days before merging
sanket1729:
ACK be553217f1
Tree-SHA512: 4df9db179a69c9eea2940008c9eca9f9b9b9d9f27e2a174db39bfb3d5c32a9d06b26e5349e5baf5af78600ae128d1b534559baf7335aec6df1238de96a508764
In #2646 we introduced a bug in the taproot witness stack getter
functions, of which we have three:
- `tapscript`
- `taproot_control_block`
- `taproot_annex`
Each returns `Some` if a possible bytes slice is found (with no other
guarantees).
Use `taproot_annex` combined with getters from `primitives` to implement
the other two getters. This simplifies the code and fixes the bug.
Add an additional getter to `primitives` `Witness::third_from_last`.
Fix: #3598
We are trying to make it so that what ever crate a user uses they see
the same module/type tree (if the module or type exists).
E.g., one can do either of these.
If they use `bitcoin`:
```
use bitcoin::{
amount, block, fee_rate, weight, merkle_tree, opcodes,
pow, script, sequence, transaction, witness,
};
```
Or if they use `primitives`:
```
use bitcoin_primitives::{amount, block, fee_rate, weight};
```
The above imports were tested and `amount` was found to be missing.
There is a range of different wordings used in the docs of constructor
type functions.
Change all to start with `Constructs a new` or `Constructs an empty`.
In functions that act like constructors there is a mixture of the usage
of `creates` and `constructs`.
Replace all occurrences of `creates` with `constructs` in the first line
of docs of constructor like functions.
The `Transaction` type was created way back when, and the field names
were named using singular (`input` and `output`). In hindsite plural
probably should have been used since its more idiomatic Rust but the
fields have been around so long now that re-naming them is going annoy a
whole bunch of people.
As a compromise add getters that use plural, immutable and mutable
versions, for both the `input` and `output` fields.
Close: #822
Is there any advantage trying to lay out the re-exports to give users an
idea of the crate structure?
We have the explicit aim that users who depend on `bitcoin` do not ever
need to reach directly into `primitives` (or `units`) however it is kind
of nice to know where things come from, saves jumping to multiple files
looking for them (for those of us that jump to files manually).
I do not know how all the re-exports interact with other folks IDEs, I
personally open files manually and just remember where stuff is.
85942c355d Re-export block::Header as BlockHeader (Tobin C. Harding)
Pull request description:
For users who want to just grab stuff from the crate root it makes total sense for there to be a `BlockHeader`.
Another nice thing, in the HMTL docs it makes BlockHeader be in the struct list right along with `Block`, `BlockHash`, and `BlockHeight`.
Close: #3548
ACKs for top commit:
apoelstra:
ACK 85942c355dfdcf9ddf9a46202f4db56794dcc85d; successfully ran local tests
jamillambert:
ACK 85942c355d
Tree-SHA512: 6fbaf7936062323d31940179ffcbc62951b21f53955a42dca2e28476bda0cebeb041db0e1b4f1ceeac32e43d94fe5476ef055595ae88dc6cfc266d32082bdf4d
During move of code to `primitives` we removed a few links to types that
were not yet moved, we can now put these back in.
Feature all rustdoc imports on `alloc` and `doc`.
Close: #2997
For users who want to just grab stuff from the crate root it makes total
sense for there to be a `BlockHeader`.
Another nice thing, in the HMTL docs it makes BlockHeader be in the
struct list right along with `Block`, `BlockHash`, and `BlockHeight`.
2b7cb04265 primitives: Add missing re-exports (Tobin C. Harding)
Pull request description:
Add re-exports of types from `taproot` and `merkle_tree`.
ACKs for top commit:
jamillambert:
ACK 2b7cb04265
apoelstra:
ACK 2b7cb04265cdb2d62c1bcc645e19d8bafa132b4b; successfully ran local tests
Tree-SHA512: e837c8e7bfda3e8c5510a15ba19b2b0b433e9e87c961ab9ab81d134288b70bc98364007e8e610e7066732e16f72ea990a782cade9ead6b71ddc9b843753fa0ee
This has been fixed and we use nightly to lint so we have access to the
merged fix.
Removing the attribute uncovers a bunch of real lint warnings, fix
them while we are at it.
a51768af3f key: Deprecate to_bytes (Tobin C. Harding)
3af3239ad0 script: Re-order functions (Tobin C. Harding)
db40297f87 script: deprecate to_bytes (Tobin C. Harding)
c5cd0db493 Revert the change to to_bytes (Tobin C. Harding)
dc2ca785d2 Add to_vec and deprecate to_bytes for array types (Tobin C. Harding)
a6b7ab32a8 Move impl_array_newtype to internal_macros (Tobin C. Harding)
Pull request description:
Use `to_vec` and deprecate `to_bytes`, the opposite of what we did in #2585.
For functions that return a `Vec` by first allocating use function name `to_vec`. This explicitly excludes:
- Functions that return an array (`CompressedPublicKey::to_bytes`)
- Functions that consume self and return a `Vec` without allocating (`ScriptBuf::into_bytes`)
See #3025 for discussion and consensus.
Close: #3025
ACKs for top commit:
apoelstra:
ACK a51768af3f3d4c8e138e1ded250800810bedc903; successfully ran local tests
Tree-SHA512: ee932c13ad2e09c2b76a7833b23c859df175aa307f56e673921f3ae8b5d865518c6f999749e3b627594457b3ca33301b777177ada3520cf006acc0f14e5dacf8
0bf1910980 Remove wildcard from script self re-exports (Tobin C. Harding)
397a4b9382 Remove wildcard in push_bytes module (Tobin C. Harding)
Pull request description:
We thought #3436 was done (well I thought it was), turns out I was wrong.
ACKs for top commit:
jamillambert:
ACK 0bf1910980
apoelstra:
ACK 0bf1910980a13005496244ec4d4adf0553afbc73; successfully ran local tests
Tree-SHA512: 131a4aa4a907099790d14cfc2d19305943a2270cf6852c7dd92e35ea69188c9a40797fb22fd2ed8b2fefc2d6380b884401b5e32f521711f0f7b2da610d0e332f
We want our code to be easy to read and our APIs discoverable, for those
of us who read source files the layout matters.
Put the constructors and getters at the top of the impl block.
Code move only, no logic changes.
Deprecate the `Script::to_bytes` function in favour of `to_vec` as we
are doing elsewhere.
Note that `ScriptBuf` has `into_bytes` because it does not copy.
Potentially this should be deprecated in favour of `into_vec`?
Note that in regards to the `to_` prefix this naming as valid according
to convention because the `Script` type is borrowed and `to_vec` copies
the underlying bytes.
During this release cycle we deprecated `to_vec` in favour of
`to_bytes`, we have since reversed our position on the name.
Remove the deprecation of `to_bytes` from the three types that had it
and use `to_vec`.
In the `script` module remove the wildcards and re-export stuff from
`self` explicitly in both `primitives` and `bitcoin`.
Internal change only, everything is re-exported.
Currently we feature gate code within the `impl_to_hex_from_lower_hex`
macro on "alloc" but `bitcoin` does not have the "alloc" feature so
this code is never built in. This can be seen by the lack of a
`to_hex` function on `LeafVersion`.
Remove the feature gate from the macro and put it on the individual
call sites as needed.
66da2266e2 Explicitly re-export stuff from crates down the stack (Tobin C. Harding)
Pull request description:
Up until recently we were using wildcard re-exports for types moved to `units` and `primitives`. We have decided against doing so in favour of explicit re-exports.
Audit `units` and `primitives` using `git grep 'pub enum'` (and `struct`) and explicitly re-export all types.
Remove all wildcards except for the re-exports from `opcodes`, there are too many opcodes, explicitly re-exporting them does not aid clarity.
ACKs for top commit:
apoelstra:
ACK 66da2266e26dfe53947c4606e9d18620931e93cf; successfully ran local tests
Tree-SHA512: 74717f8b127e975e3d131aab884bdfe78e699d88b7ee1db7731ad117437d37684285264001cf6b2182eb1e565171167695e00c4b6aef28a3e26b69d9cebfbb74
Up until recently we were using wildcard re-exports for types moved to
`units` and `primitives`. We have decided against doing so in favour of
explicit re-exports.
Audit `units` and `primitives` using `git grep 'pub enum'` (and
`struct`) and explicitly re-export all types.
Remove all wildcards except for the re-exports from `opcodes`, there are
too many opcodes, explicitly re-exporting them does not aid clarity.
4a8e2c3704 Add tests for sequence (Shing Him Ng)
Pull request description:
Add a few unit tests for Sequence
ACKs for top commit:
apoelstra:
ACK 4a8e2c3704 successfully ran local tests; sure
tcharding:
ACK 4a8e2c3704
Tree-SHA512: fb793a6a94a9d3f1522357f98a73a87618ba70776875b3ce051631970ca6c15998401dece47e426f7040770b8d24ee1b72f206d3164b7ca76492e12e86116677
c1eccfde25 Move Witness to primitives (Tobin C. Harding)
6ce76cd7c8 Add unstable constructor to Witness (Tobin C. Harding)
Pull request description:
Patch 1 introduces a new policy to the codebase, we use `foo__unstable` for public unstable functions and there are zero semver guarantees if you call these functions.
Patch 2 does the move.
Close#3406
ACKs for top commit:
apoelstra:
ACK c1eccfde25fd4c2b19e7ec6759352b46ac246113; successfully ran local tests
Tree-SHA512: 2388066be2b6bb2cf3d6757c8f6334beeda6115ef1ce7c537955d32aa5e466add5162d0d2adee27f416fe622fc93c4e94bd848326463ee55e08d1c0f4e03719c
Move the `Witness` over to `primitives` leaving behind any method that
takes or returns a `Script` or a signature.
Includes addition of a feature gate to unit test.
ff64fa3c46 Move block::Header to primitives (Tobin C. Harding)
65925117a0 Manually implement block::Header::block_hash (Tobin C. Harding)
b02ab25342 Add regression test for block_hash function (Tobin C. Harding)
5614694b69 block: Remove wildcard re-export (Tobin C. Harding)
Pull request description:
- Patch 1: Trivial preparation.
- Patch 2 and 3: Reduce the API surface in a disappointing way due to the orphan rule.
- Patch 4: Do the move.
ACKs for top commit:
apoelstra:
ACK ff64fa3c46 successfully ran local tests; nice!
Tree-SHA512: 8d93de4a12c9f71f9dfbdc023fd8defb78f7d3da995490af8f83a927c2ca6338dd236bc08691311dc7345183fb6d0be61eedce3e4424d4060be2de934facd60a
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`.
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
3565f70df9 feat: replace ENABLE_RBF_NO_LOCKTIME with ENABLE_LOCKTIME_AND_RBF (ChrisCho-H)
Pull request description:
follow up https://github.com/rust-bitcoin/rust-bitcoin/pull/3455.
Replace all `ENABLE_RBF_NO_LOCKTIME`(deprecated) with `ENABLE_LOCKTIME_AND_RBF`
ACKs for top commit:
apoelstra:
ACK 3565f70df9 successfully ran local tests
tcharding:
ACK 3565f70df9
Tree-SHA512: 4c6c6ad9ac89efe042cf239ce68594e715dbc827c1ae430819e2f16d2191f82d81f1e55b348f8176fc21454cf9d25fc9da1a414982cd3483d5f9f39834441f6c
Tobin C. Harding
merge-script
Jamil Lambert, PhD
Fmt Bot
Leo Nash
Shing Him Ng