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
37cd6503f1 ci: Remove TOOLCHAIN env var (Tobin C. Harding)
8eb6c23eaf ci: Remove inner quotes from shell variable (Tobin C. Harding)
58ede47ef1 ci: Use set -ex instead of /bin/sh -ex (Tobin C. Harding)
4c4846f82a Remove double blank line at EOF (Tobin C. Harding)
Pull request description:
Clean up the CI script. These patches are part of #1225, are beneficial to have before that PR resolves, and are trivial.
ACKs for top commit:
sanket1729:
utACK 37cd6503f1
apoelstra:
ACK 37cd6503f1
Tree-SHA512: f6b3379c8a1be7a3519461faeed35e35560e4228bddb59ef28ab3fb3ef2679afabfaccfeaffd78adbb66879aed02c092f3d9d1d1eb3193c811067ee75e876732
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
With the latest release of `rust-bitcoinconsensus` the
`bicoinconsensus::Error` now implements `std::error::Error`.
Correctly handle `bicoinconsensus::Error` by returning `Some(e)` and
using `write_err` as is standard across the code base.
fea908c8af Add private::Sealed trait bound to SerdeAmount (Tobin C. Harding)
08e4b28dd0 Add integer serialization tests (Tobin C. Harding)
a8f9e8ad96 Add serde roundtrip tests (Tobin C. Harding)
Pull request description:
Remove some easy todos from the code. The three patches are unrelated except each removes a single `TODO`. Patch 1 and 2 are unit tests and trivial to review. Patch 3 requires more thought, it was't clear exactly why it was left as a todo and not done at the time.
cc JeremyRubin for patch 3 as the original author of that code.
ACKs for top commit:
apoelstra:
utACK fea908c8af
Tree-SHA512: 70a041820fe52bc3d70cc110c16acfa86469fd8556793c4204671ddcb7457f336e57b43b0f03cb68d3c6b96217502f29f1628d1acbe7381a9e9d8d21c67759d2
As suggested by the todo seal the `SerdeAmount` so users of the library
explicitly cannot implement it.
Its not obvious to me why this wasn't done at the time.
Remove the associated TODO from the code.
Add some negative integer unit tests and remove the TODO.
Note, this is not really that necessary because we are going to move to
using the stdlib `to_le_bytes` methods so we don't really need any of
these test except to test that we are calling the correct be/le method.
They do no harm however.
pmt_tests with different tx counts were executed in a single test, not taking advantage of the parallelism provided by unit tests.
A macro was added to define a unit test per transaction count.
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.
There were some duplicated hex macros in the `address` module as well as
several macros outputing specific types instead of working with any type
deserializable from hex.
This cleanup moves the macros and deduplicates them, introduces a new
`TestFromHex` trait to generalize parsing hex and rewrites the code to
use hex parsing generally. `hex_script` is still kept because its usage
is very frequent.
Several macros used in tests had `#[cfg(test)]` condition on each of
them. This deduplicates them by putting them into a separate module and
reexporting (with one condition for module and one for re-export).
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.
4c1dc4594d Implementing a macro to define debug from display and adding Display trait implementation where needed (hrouis)
Pull request description:
Related to issue : [1201](https://github.com/rust-bitcoin/rust-bitcoin/issues/1201)
> Replaced macro **impl_display_from_debug** with **impl_debug_from_display**
> Added implementation of Display trait where needed.
ACKs for top commit:
Kixunil:
ACK 4c1dc4594d
apoelstra:
ACK 4c1dc4594d
Tree-SHA512: a240ccd507ac957ed82adf14623d2b77ee7a19458c2617b69f440eba8ebe52d4b45c1328198116ee720aeee418f6390238772eed002f850cc1bfbdc0248b606e
Display should not be implemented as Debug trait, but the existing macro promotes this figure of use.
The macro has been replaced with implement_debug_from_display and the Display trait has been implemented where needed.
The LowerHex trait has been implemented instead of Display for UintX types.
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.
6095a4de9b Use blockdata exports (Tobin C. Harding)
4a119e5624 Re-export blockdata modules (Tobin C. Harding)
Pull request description:
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.
ACKs for top commit:
apoelstra:
ACK 6095a4de9b
sanket1729:
ACK 6095a4de9b
Tree-SHA512: d9e0af1fa237bf9ad730c40088fe951dbc97df5ef9407a7038c6dbeb2f7450b93b082a40d76f5c5559731ac530fd9849405afae090de9a565e312ac4951cc0d6
34f955a073 Introduce new canonical license blurb (Tobin C. Harding)
Pull request description:
Recently we introduced SPDX license ids but there was some confusion
about the use, form, and purpose of lines before (library name, author).
It was pointed out to me by Andrew that if/when folks cut'n'paste our
code they often will keep the whole blurb. Design it with that in
mind. FTR, Andrew also indicated that he did not mind his name being
removed in favour of "The rust-bitcoin developers".
At first I thought the term "The rust-bitcoin developers" was a bit
wishy-washy but yesterday I realised that I am proud to be part of that
crew, striving to deliver code to the highest possible standard.
Introduce a canonical format for the licences blurb.
- Use the name of the library
- Use "The rust-bitcoin developers"
- Use the SPDX ID
ACKs for top commit:
apoelstra:
ACK 34f955a073
Kixunil:
ACK 34f955a073
Tree-SHA512: d488bebe9f7a862aa353b1f15c29c47579715d7a9a35120d4b4a5846f0100f4be3e53e2a412f0edea426c276f39740bbf07771ee51e0532951403168ba2b1224
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
abc014a079 Move rustdoc above attribute (Tobin C. Harding)
e9230019eb Implement std::error::Error::source for MerkleBlockError (Tobin C. Harding)
613f1cf2d5 Implement std::error::Error::source for bip152 (Tobin C. Harding)
a37ab82503 Add non_exhaustive to _all_ errors (Tobin C. Harding)
Pull request description:
Do error cleanups, and add a script to help find missing `non_exhaustive` on error types
- Patch 1: Audit the codebase and put `non_exhaustive` on all error types.
- Patch 2: Implement `std::error::Error::source` for `bip152::Error`
- Patch 3: Implement `Display` and `std::error::Error::source` for `MerkleBlockError`
- Patch 4: Move rustdocs to above attributes on one error type
- ~Patch 5: Add a python script to `contrib` that checks the codebase for missing non exhaustive on the line above the results of regex `pub enum .*Error`~
I removed the Python script patch, I can't be bothered working on this for now but the clean ups and `non_exhaustive` additions are useful IMO.
### labels
- I added 'API break', I think the `non_exhaustive` addition requires major bump but not sure?
- release notes mention is just for the `non_exhaustive` patch.
ACKs for top commit:
Kixunil:
ACK abc014a079
apoelstra:
ACK abc014a079
Tree-SHA512: 16ea15014eae97de7ac9cca1e9b76304aa3702a98cde577c2d71343022f840d3b33a39d2ab6d3fba0f0f1ebaa1631a0595eea1d794ba9727fe6ccfcf08e2feae
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.
Clean up some imports, only do the ones that we are about to touch.
- Remove/add whitespace to group by crate
- move modules/types to their module instead of using lib wide
re-export (this helps minimise future merge conflicts)
Re-order imports, 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.
We have a bunch of clippy warnings on master, at least one of them is
from a patch dated from March, before we ran clippy on CI.
Fix clippy warnings
- warning: accessing first element with `self.0.get(0)`
- warning: deref on an immutable reference
- warning: you are deriving `PartialEq` and can implement `Eq`
All one patch because clippy has to run cleanly for each patch on CI
now.
Please note the `Eq` change, adding this derive is an API change.
dfb8ef97dc Remove deprecated MerkleBlock methods (Tobin C. Harding)
b171f88bc3 Remove deprecated SighashComponents struct (Tobin C. Harding)
c2bbf7c343 Use SighashCache instead of SigHashCache (Tobin C. Harding)
1c04a99ec3 Use sighash::SighashCache for bip143 unit tests (Tobin C. Harding)
f65e034742 Refactor test module imports (Tobin C. Harding)
Pull request description:
I'm not sure what the exact policy we decided on for deprecating is but we have code deprecated since v0.24.0 and v.026.2, both of these seem reasonable safe to remove.
Removal of `SighashCompontents` uncovered the fact that we never moved the BIP 143 test vector code over to the new `SighashCache`.
- Patch 1: Trivial preparatory cleanup
- Patch 2: Re-write bip143 test vectors using `sighash::SighashCache`
- Patch 3: Trivial, use `SighashCache` instead of `SigHashCache`
- Patch 4: Remove `SighashComponents` struct deprecated in 0.24.0
- Patch 5: Remove `MerkleBlock` methods deprecated in 0.26.2
ACKs for top commit:
apoelstra:
ACK dfb8ef97dc
sanket1729:
ACK dfb8ef97dc
Tree-SHA512: 493525859a131307fbe3ea17f8edb70045dc56526d5579a4fc0d6e7c3fded58c890304721502d1cfceed0a61f4b3d1827a60760e42af0114f3b3ba29b136238c
c062e4ff80 Deprecate legacy sighash methods (Tobin C. Harding)
1fdf5f9b82 Move legacy sighash unit test data to test_data (Tobin C. Harding)
6fa0329930 Re-order import statements (Tobin C. Harding)
Pull request description:
When we introduced the `SighashCache` we put encoding and sighash code
for segwit (v0 and taproo) in the `sighash` module. We also added
methods for legacy inputs that wrapped calls to encode/sighash methods
in the `transaction` module. This is confusing for a couple of reasons
- When devs read `Transaction` there are two methods that apparently
enable encodeing tx data and calculating the sighash but there is no
indication that these methods are only for legacy inputs.
- Once devs work out that segwit inputs have to be handled by methods on
the `SighashCache` it is not obvious why the methods on `Transaction`
exist at all.
Move the legacy encode/sighash code over to the `sighash` module and
deprecate the old methods. (Includes moving unit tests.)
Patch 1 and 2 are preparation.
(I did this work because I got lost in `Transaction::signature_hash` when messing with `TxOut::Amount` and trying to understand the signing algorithm vs bip143.
sanket1729 is there some other reason that I'm missing why we left the legacy code in `transaction`?
ACKs for top commit:
sanket1729:
ACK c062e4ff80
apoelstra:
ACK c062e4ff80
Tree-SHA512: b94de928e64e652ec75e34af94d5d161f5c52a862b5b10904e980b8ae669b3b434f8639d5ddadad84a7bbdf36ac21bdd287398448e844b5bb531c3664e91fb02
When we introduced the `SighashCache` we put encoding and sighash code
for segwit (v0 and taproo) in the `sighash` module. We also added
methods for legacy inputs that wrapped calls to encode/sighash methods
in the `transaction` module. This is confusing for a couple of reasons
- When devs read `Transaction` there are two methods that apparently
enable encodeing tx data and calculating the sighash but there is no
indication that these methods are only for legacy inputs.
- Ocne devs work out that segwit inputs have to be handled by methods on
the `SighashCache` it is not obvious why the methods on `Transaction`
exist at all.
Move the legacy encode/sighash code over to the `sighash` module and
deprecate the old methods. (Includes moving unit tests.)
Tobin C. Harding
Andrew Poelstra
Noah Lanson
hrouis
Martin Habovstiak
sanket1729