947dcf5675 Add new type for block version (Noah Lanson)
Pull request description:
Added new type `BlockVersion` for block header version field with inspiration from [Kixunil 's comment on #1215.](https://github.com/rust-bitcoin/rust-bitcoin/issues/1215#issue-1352273532)
I previously knew very little about the upgrading process so it was fun learning about it in writing this PR, however that also means it's probably not perfect yet, especially around BIP-9 signalling (fingers crossed it's all correct).
API for the type is kept simple but happy to change it up to add more functionality.
ACKs for top commit:
sanket1729:
ACK 947dcf5675.
apoelstra:
ACK 947dcf5675
Tree-SHA512: 1ce9720d50f0ab50e08308e56bdc047567b64dbe446dcdcd9db2f14f5c9d4603a7b9d58a5fa38b769581789fdbc4e1ef6083be32b9b59cef59714e07b2f8be5f
01161e66ee Run formmater on bip158 (Tobin C. Harding)
95393aadbc Move bip158 module to crate root (Tobin C. Harding)
Pull request description:
We are attempting to flatten the `util` module.
Move the `bip158` module to the crate root out of `util`.
Currently `src/util/` is ignored by the formatter so this move requires `bip158` module to be formatted. Formatting is done as a separate patch so reviewers can run `cargo +nightly fmt` and compare the diffs if so desired.
ACKs for top commit:
apoelstra:
ACK 01161e66ee
sanket1729:
ACK 01161e66ee
Tree-SHA512: 192279d8d1466aa939ecad1f7beae12c4c7b5067871f6f1297fc80094a4ab736de4c1ed82a0b1d9d8e1cdba15c0342b90722002a9ba02ab1eff901edbd0fb356
We recently added the `Sequence` type and we explicitly did not include
relative lock time logic.
Add a new module `relative` and new type `LockTime` to represent
relative lock times as defined by BIP 68.
We are attempting to flatten the `util` module; move the `bip158` module
to the crate root out of `util`.
Currently `src/util/` is ignored by the formatter so this move causes
the `bip32` module to be formatted.
In preparation for adding a relative lock time type move the `locktime`
module to a new module called `absolute`. Use qualified path for
locktime types (e.g. `absolute::PackedLockTime`) to improve readability.
In an effort to make the library more ergonomic to use re-export modules
from `blockdata` at the crate root level. This helps to decouple the
internal code layout with the public API.
cc2c67fc2d Move sighash types to the sighash module (Tobin C. Harding)
7c040f4437 Refactor imports (Tobin C. Harding)
29f21d6ff5 Add additional sighash related pubic exports (Tobin C. Harding)
Pull request description:
This is non-urgent work, something I've wanted to do for months now, please only review at your leisure :)
Currently we have a bunch of sighash types defined in the `transaction` module and we have a newer `sighash` module that defines a bunch of related types. This is an artifact of the development process as we added the new types. If we put things that are related together it makes it easier for devs to find their way around the library and grok the code.
Move all the sighash types from the `blockdata::transaction` module to the `sighash` module.
The first two patches are preparatory cleanup.
### Labels
I added "API break" because I move the types without adding a re-export from the original place. I believe this is correct because we are, hopefully, aiming to de-couple the public API from the internal module structure so there is going to be a load more of this.
ACKs for top commit:
apoelstra:
ACK cc2c67fc2d
sanket1729:
ACK cc2c67fc2d
Tree-SHA512: 62a586b63c388baf91655a9afc8595394565d6ea6eb9aa3bd4746f899c140f332999e886cd85b098ec01304d2b96a310848d42d5ae38af476d3a26496576e36c
Currently we have a bunch of sighash types defined in the `transaction`
module and we have a newer `sighash` module that defines a bunch of
related types. This is kruft from the development process as we added
the new types. If we put things that are related together it makes it
easier for devs to find their way around the library and grok the code.
Move all the sighash types from the `blockdata::transaction` module to
the `sighash` module.
Refactor only, no logic changes.
We already export some of the sighash related types, there are others
that are uniquely named that can also be exported. Doing so makes use of
the library more ergonomic because devs do not have to know where types
are defined.
We just moved the `address` module out of `util` which is currently
ignored by `rustfmt`.
Run `rustfmt`, no other changes other than those made by the tool.
The identifier 'util' does not convey any information. We have a whole
bunch of modules inside the `util` module.
As part of work to reduce the amount of arbitrary things in the `util`
module move the `address` module to the crate root level.
We have formatting issues on master because we do not yet run the
formatter in CI (PR is open).
We are about to move the `address` module to `/src` which will require
running the formatter.
Run `cargo fmt` to fix formatting issues, no other changes.
When debugging parsing errors it's very useful to know some context:
what the input was and what integer type was parsed. `ParseIntError`
from `core` doesn't contain this information.
In this commit a custom `ParseIntError` type is crated that carries the
one from `core` as well as additional information. Its `Display`
implementation displays this additional information as a well-formed
English sentence to aid users with understanding the problem. A helper
function parses any integer type from common string types returning the
new `ParseIntError` type on error.
To clean up the error code a bit some new macros are added and used.
New modules are added to organize the types, functions and macros.
Closes#1113
Add a `LockTime` type to hold the nLockTime `u32` value. Use it in
`Transaction` for `lock_time` instead of a `u32`. Make it public so this
new type can be used by rust-miniscript and other downstream projects.
Add a `PackedLockTime` type that wraps a raw `u32` and derives `Ord`,
this type is for wrapping a consensus lock time value for nesting in
types that would like to derive `Ord`.
For internal macros used only in this crate we do not need to use
`macro_use` and pollute the top level namespace now that we have edition
2018. We can add a `pub(crate) use` statement to each and then path
imports work for the macros like normal types.
Now we have edition 2018 we do not need to use `macro_use` or `extern
crate`; `pub use` works with macros. Notable exceptions are `alloc` and
`test`. Also leave the serde rename because touching it opens a can of
worms.
Recently we removed the "unstable" feature, I missed the duplicate
`extern crate test` when doing so :(
Since we no longer have the "unstable" feature this line of code is
never compiled in.
Remove the unused ``extern crate test`, we have the correct line further
up the file `#[cfg(bench)] extern crate test;`.
Now we have edition 2018 we do not need to use `macro_use` or `extern
crate`; `pub use` works with macros.
Remove the extern crate statement and replace it with a pub use
statement. Requires fixing up various imports statements and also
requires importing `sha256t_hash_newtype` macro directly instead of
using a qualified path to it.
In preparation for running the formatter on all source files at the root
of the crate; skip formatting `prelude` module because we use
unconventional import statements so they to save writing a million
compiler attributes.
e34bc538c3 Add new type for sequence (Noah Lanson)
Pull request description:
#1082
Created a new type for txin sequence field with methods to create sequences with relative time locks from block height or time units.
ACKs for top commit:
Kixunil:
ACK e34bc538c3
tcharding:
ACK e34bc538c3
apoelstra:
ACK e34bc538c3
Tree-SHA512: 6605349d0312cc36ef9a4632f954e59265b3ba5cfd437aa88a37672fe479688aa4a3eff474902f8cc55848efe55caf3f09f321b3a62417842bfc3ec365c40688
f3b2120ec9 Create configuration conditional bench (Tobin C. Harding)
f60c92ca58 Add informative error message to DO_BENCH (Tobin C. Harding)
c6d5a12b60 Add cargo/rustc sanity calls (Tobin C. Harding)
34d5a3141d Put triple ticks on their own line (Tobin C. Harding)
Pull request description:
Currently we are unable to build with all features enabled with a non-nightly toolchain, this is because of the use of
`#![cfg_attr(all(test, feature = "unstable"), feature(test))]`
which causes the following error when building:
error[E0554]: `#![feature]` may not be used on the stable release channel
The "unstable" feature is used to guard bench mark modules, this is widely suggested online but there is a better way.
When running the bench marks use the following incantation:
`RUSTFLAGS='--cfg=bench' cargo bench`
This creates a configuration conditional "bench" that can be used to guard the bench mark modules.
```
#[cfg(bench)]
mod benches {
...
}
```
ACKs for top commit:
Kixunil:
ACK f3b2120ec9
apoelstra:
ACK f3b2120ec9
Tree-SHA512: 7ec2a501a30bfe2ce72601077cd675cf5e5ac2f0f93f97fc7e83cb7401606b69ae909b35bfc0ace8bd1ea771ca4fba70e2ad9ac9ba26f2b6e371494cf694c0a8
Currently we are unable to build with all features enabled with a
non-nightly toolchain, this is because of the use of
`#![cfg_attr(all(test, feature = "unstable"), feature(test))]`
which causes the following error when building:
error[E0554]: `#![feature]` may not be used on the stable release
channel
The "unstable" feature is used to guard bench mark modules, this is
widely suggested online but there is a better way.
When running the bench marks use the following incantation:
`RUSTFLAGS='--cfg=bench' cargo bench`
This creates a configuration conditional "bench" that can be used to
guard the bench mark modules.
#[cfg(bench)]
mod benches {
...
}
91ff2f628c Introduce SPDX license identifiers (Tobin C. Harding)
Pull request description:
When `rust-bitcoin` was started in 2014 the SPDX license list and short identifiers where not a thing. Now that we have short identifiers and they are gaining popularity in other projects we can consider using them.
- Add links to the SPDX website in the readme
- Shorten the author section to a single line
- Remove all the licence information in each file and replace it with an
SPDX ID (see https://spdx.dev/ids/#how)
Of note:
- If the author of a file is explicitly listed, maintain this information
- If the 'author' is listed as the generic 'Rust Bitcoin developers' just remove the attribution, this is implicit. This does loose the date info but that can be seen at any time from the git index using
`git log --follow --format=%ad --date default <FILE> | tail -1`
apoelstra, please confirm that I'm not treading on your toes here, especially, are you ok with the new 'written by' string format?
### Ref
- https://spdx.dev/ids/#how
- https://spdx.org/licenses/CC0-1.0.html
- https://spdx.dev/ids/
ACKs for top commit:
apoelstra:
ACK 91ff2f628c
sanket1729:
ACK 91ff2f628c. I am also in IDGAF camp, but I like more red lines in diff.
Kixunil:
ACK 91ff2f628c
Tree-SHA512: ca8aac00f015c18ec18de83dfeb50dd6f4f840653c7def85daa2436a339021ada5f3c34ad0cdf6b18e3e39c45a6d58a8313742e4001d467785b10eee7fdbc938
When `rust-bitcoin` was started in 2014 the SPDX license list and short
identifiers where not a thing. Now that we have short identifiers and
they are gaining popularity in other projects we can consider using
them.
- Add links to the SPDX website in the readme
- Shorten the author section to a single line
- Remove all the licence information in each file and replace it with an
SPDX ID (see https://spdx.dev/ids/#how)
Of note:
- If the author of a file is explicitly listed, maintain this
information
- If the 'author' is listed as the generic 'Rust Bitcoin developers'
just remove the attribution, this is implicit. This does loose the date
info but that can be seen at any time from the git index using
`git log --follow --format=%ad --date default <FILE> | tail -1`
The docs lint `broken-intra-doc-links` has been changed but the new name
is not available in our MSRV, this means we get a build warning. We only
build docs with the nightly toolchain so we can move this lint control
to the docs build command in `test.sh` instead of doing it crate wide.
With this patch applied devs risk not noticing docs link issues until
they hit them on CI _if_ they do not build with the test script or
explicitly pass in `-- -D rustdoc::broken-intra-doc-links`, which no one
is going to do. Hence we add a line to the readme with a shell alias
that can be used to check docs, taken directly from `test.sh`.
Features activating external crates are supposed to have same name as
those crates. However we depend on same feature in other crates so we
need a separate feature. After MSRV bump it is possible to rename the
crates and features so we can now fix this inconsistency.
Sadly, derive can't see that the crate was renamed so all derives must
be told to use the other one.
Use cargo to upgrade from edition 2015 to edition 2018.
cargo fix --edition
No manual changes made. The result of the command above is just to fix
all the use statements (add `crate::`) and fix the fully qualified path
formats i.e., `::Foo` -> `crate::Foo`.
Our usage of `SigHash` implies that 'sighash' is _two_ words; 'sighash'
is a well known word in the Bitcoin ecosystem it should appear in
identifiers as `Sighash`.
Rename `SchnorrSigHashType` to `SchnorrSighashType`.
Our usage of `SigHash` implies that 'sighash' is _two_ words; 'sighash'
is a well known word in the Bitcoin ecosystem it should appear in
identifiers as `Sighash`.
Rename `EcdsaSigHashType` to `EcdsaSighashType`.
We currently have the `map` module private but containing a bunch of
types that are needed in the public API (specifically in a
`PartiallySignedTransaction`).
Re-export the publicly required types to the `psbt` module and then
again at the root level of `rust-bitcoin` as we do for other types.
Using the latest version of rust-bitcoin master on rust-miniscript
errors on bitcoin::SigHashType not found. In the original PR, I only
renamed the export to ECDSASigHashType, but original re-export should
also be there in lib.rs to avoid to breaking changes downstream.
Docs can always do with a bit of love.
Clean up the module level (`//!`) rustdocs for all public modules.
I claim uniform is better than any specific method/style. I tried to fit
in with what ever was either most sane of most prevalent, therefore
attaining uniformity without unnecessary code churn (one exception being
the changes to headings described below).
Notes:
* Headings - use heading as a regular sentence for all modules e.g.,
```
//! Bitcoin network messages.
```
as opposed to
```
//! # Bitcoin Network Messages
```
It was not clear which style to use so I picked a 'random' mature
project and copied their style.
* Added 'This module' in _most_ places as the start of the module
description, however I was not religious about this one.
* Fixed line length if necessary since most of our code seems to follow
short (80 char) line lengths for comments anyways.
* Added periods and fixed obvious (and sometimes not so obvious)
grammatically errors.
* Added a trailing `//!` to every block since this was almost universal
already. I don't really like this one but I'm guessing it is Andrew's
preferred style since its on the copyright notices as well.
This clearly states lack of support for 16-bit architectures as well as
adds readable `compile_error!()` call. It also fixes a few stylistic
mistakes - headings (top-level should not be repeated) and missing
newlines.
Closes#660
Andrew Poelstra
Noah Lanson
Tobin C. Harding
sanket1729
Martin Habovstiak
junderw
Dr Maxim Orlovsky