ee19a1633d internals: Run formatter (Tobin C. Harding)
Pull request description:
Add one `rustfmt::skip` statement and run `cargo +nightly fmt`.
ACKs for top commit:
apoelstra:
ACK ee19a1633d
Kixunil:
ACK ee19a1633d
Tree-SHA512: 524ff2cda383c95a086f4b2e9da443bf0202fc21f71024e6230d64ad57098ab304402dd12c9e4d831b3ef5c153f45e916ae4c928107ac9a847fba65354365a98
8aa94bd0b2 Improve docs on is_implied_by (Tobin C. Harding)
b8721bf244 Add method relative::LockTime::is_implied_by (Tobin C. Harding)
d5492b8a25 Add absolute::LockTime::is_implied_by method (Tobin C. Harding)
98cbdb5a5c Increment lock value (Tobin C. Harding)
Pull request description:
Patch 1 is a docs improvement.
Patch 2 commit log:
When implementing the absolute lock time API we decided on _not_
supporting checking lock satisfaction with another lock, instead we
provided a pattern in the docs for doing so. Fast forward a months and
I, the primary author, then forgot to use the correct pattern when using
the API in `rust-miniscript` - this is a sure sign that the API is too
hard to use. In this time we worked on the relative lock API and came up
with a `is_satisfied_by_lock` method - this is identical to the required
use case in the absolute lock time module.
Add a method on `absolute::LockTime` for checking a lock against another
lock, add rustdoc comment explaining the methods function in filtering
prospective lock time values (how we use it in `rust-miniscript`).
ACKs for top commit:
Kixunil:
ACK 8aa94bd0b2
apoelstra:
ACK 8aa94bd0b2
Tree-SHA512: 5c7efa1727a846248783c9e6044bf8b0a7550d298ca1b5d3274ef325cf82efa33392ad14ef7e3e9aa91423ba56e8a3e7f4a38a966be38f673dccefd46465ad51
f429c22599 Remove user_enum macro (Tobin C. Harding)
ad29084582 Add Network serde roundtrip test (Tobin C. Harding)
Pull request description:
Remove the `user_enum` macro because it is only used once and is unnecessarily complicated.
Patch 1: Add a preparatory unit test to make sure patch 2 maintains the current serde logic.
Patch 2: Simplify `Network` by removing `user_enum`
ACKs for top commit:
Kixunil:
ACK f429c22599
apoelstra:
ACK f429c22599
Tree-SHA512: 770ee0aa82be711ed11bdfda338de8d13071743cdc8d7e3e1252a6759dfdfa2be6ff1d8cd286acef75dc5a5d73a6bc5f565b02cb20d4b549e0714cef5d7c1973
b79c178ea8 Add fuzz test for PrefilledTransaction (Tobin Harding)
Pull request description:
Add a simple deserialization fuzz test for `PrefilledTransaction`.
Fixes: #460 (I think, I'm not sure if any of the other types need fuzzing?)
ACKs for top commit:
apoelstra:
ACK b79c178ea8
Kixunil:
ACK b79c178ea8
Tree-SHA512: 3e5cbda6b17c36f414bce05b562b5648d82433b185853e6c8aa33ec491b58e527626eb09caca2a988120329efd92dfd3aaad60ef99746d6ac41555160efeb9d6
The `user_enum` macro is only used a single time. The macro includes
custom serde logic which can be trivially derived instead.
Remove the `user_enum` macro and just implement `Network` the old
fashioned way. Doing so simplifies the code.
The lock time methods are a source of endless confusion; make an attempt
at improving further the documentation on the two `is_implied_by`
methods (one on absolute lock time and one on relative).
As we just did for `absolute::LockTime` add a method `is_implied_by` and
deprecate `is_satisfied_by_lock`.
Reasoning: it is odd to think of a lock satisfying another lock but it
is clear to see that satisfaction of one lock can imply satisfaction of
another.
92ef41b663 Make `ChainHash::using_genesis_block` constant (Jeffrey Czyz)
Pull request description:
ChainHash::using_genesis_block can't be `const` if it uses a `match` expression prior to Rust 1.46. Use an array mapping to work around this limitation.
Follow-up suggested in [#1283](https://github.com/rust-bitcoin/rust-bitcoin/pull/1283#issuecomment-1249418809).
ACKs for top commit:
apoelstra:
ACK 92ef41b663
Kixunil:
ACK 92ef41b663
Tree-SHA512: 71f95877c8e5335012ad0339e1f8691e3b33344fa02ecc24c3d4d728232cb7b0de62aec20eb1855b23eeccfbc2eeab920b21ee2243d95c6c89fa8ad5bc846975
ChainHash::using_genesis_block can't be `const` if it uses a `match`
expression prior to Rust 1.46. Use an array mapping to work around this
limitation.
96dfcdf3b7 Implement From for hash types (Noah)
Pull request description:
Reopening #1280 on the right branch + implemented `From` for references of types that were done in #1280.
ACKs for top commit:
Kixunil:
ACK 96dfcdf3b7
apoelstra:
ACK 96dfcdf3b7
Tree-SHA512: ad762032390f060b87cdd24033a5fc13a4c5297c55d7091ed89c5ca240be2f57998c0be084f40f9b04833920756b93a3ca4576a2ef944872354d1b3607734228
7a1aa2098a Remove code deprecated last release (Tobin C. Harding)
Pull request description:
We give one release cycle for deprecating old code so as to make the upgrade path easier for downstream users.
Remove code deprecated during the last release (v0.29.0).
(Check out my diff stats - all red ;)
ACKs for top commit:
apoelstra:
ACK 7a1aa2098a
Kixunil:
ACK 7a1aa2098a
Tree-SHA512: d4b9c65d0d8a0aac31cf94d826e8a6084d4f5427a26da2ad5a3973c7f5931fa10695214dc602261e9079e1c06c6dc3f5b5dcdb8d20b5c39eaadd9a33d23746dd
b1d85160ba Add constants to `ChainHash` for each `Network` (Jeffrey Czyz)
Pull request description:
`ChainHash::using_genesis_block` can't be made `const` because it uses a `match` expression, which is only valid in Rust 1.46. Add individual constants as a workaround so that `ChainHash` can be used in `const` contexts.
ACKs for top commit:
apoelstra:
ACK b1d85160ba
tcharding:
ACK b1d85160ba
Tree-SHA512: 808628e633be7db8fd3676edebefc998bba55c2434028b46873cff82fa5440b4f940904a2cb4cced23e1cabf7680234a434f823e89a13c73e52c81fd50ec312f
`ChainHash::using_genesis_block` can't be made `const` because it uses a
`match` expression, which is only valid in Rust 1.46. Add individual
constants as a workaround so that `ChainHash` can be used in `const`
contexts.
We give one release cycle for deprecating old code so as to make the
upgrade path easier for downstream users.
Remove code deprecated during the last release (v0.29.0).
3f275f7f2b Remove code deprecated in v0.28.0 (Tobin C. Harding)
c4eb218cd0 schnorr:: Remove incorrect deprecated (Tobin C. Harding)
Pull request description:
We have a bunch of things deprecated since v0.28.0, I believe we agreed to deprecate for two release cycles so this stuff can all be deleted now.
- Patch one does the `schnorr` module; the deprecated attribute usage is just plain wrong (see https://github.com/rust-bitcoin/rust-bitcoin/discussions/1275). Also, in an attempt to make the API more ergonomic, patch one adds re-exports of secp types.
- Patch 2 deletes all other code deprecated since v0.28.0
ACKs for top commit:
Kixunil:
ACK 3f275f7f2b
apoelstra:
ACK 3f275f7f2b
Tree-SHA512: 501d792c48830d28c9ad04335a56d75cf31e0bd8dc1f4e184d7d6906dbf2468369d809954709014ee5ec32ac330b56008d4be14d5f3cc89226032e642556e945
1bf885550e Redesign `hex::BufEncoder` to accept owned arrays (Martin Habovstiak)
Pull request description:
Not being able to create an owned `BufEncoder` prevented returning it from functions which need to allocate the buffer on stack. Such is the case in WIP serde via consensus serialzation.
This change refactors `OutBytes` to be unsized, adds an `AsOutBytes` trait and uses that one instead of `Into` to perform the conversion.
Closes#1270
This is meant as potentially mergeable demonstration. Interestingly, it was easier than I expected.
ACKs for top commit:
tcharding:
Except for the question about the `out_bytes` module, ACK 1bf885550e
apoelstra:
ACK 1bf885550e
Tree-SHA512: 39bc48e54ae0e66b988ba5ad6ea7ecbe03e5f4c71792df0f8e2b03aa2e97e2c0fac1cb03e84ecac12ec6f13649554b57e1000710c34c638d17d9bb575d0ac0a1
Currently we attempt to have deprecated key types in the `schnorr`
module. The `deprecated` attribute does not work on types, only on
functions.
Remove the broken deprecation logic and re-export key types instead of
using type alias', this allows a bunch of qualified paths to be
simplified also.
Add `pub use` re-exports of all secp256k1 types that are part of the
public API of the `schnorr` module. This makes the module more ergonomic
to use.
6b96050d1f Document `cfg` (Martin Habovstiak)
6fc4860813 Activate `rust_v_1_46` when on high-enough rustc (Martin Habovstiak)
Pull request description:
This fixes minor mistakes that I made in previous PR.
ACKs for top commit:
apoelstra:
ACK 6b96050d1f
tcharding:
tACK 6b96050d1f
Tree-SHA512: a926d1b642058538c3a61760593c387a9316050d7468c3eefd18c7a6c1109e736032ac49704b16f418dc0649fe110d48e188cb6b267f002ac60803f0f5a9fedb
When filtering it is necessary to check two lock times against each
other, we currently provide a patter for doing so in the docs but we can
do better.
It was observed that satisfaction of a lock time 'implies' satisfaction
of another lock time if the lock times are the same unit and one locks
value is less than the others - this is exactly the code pattern we
suggest for filtering.
Add a method on `absolute::LockTime` for checking a lock against another
lock, add rustdoc comment explaining the methods function in filtering
prospective lock time values (how we use it in `rust-miniscript`).
Not being able to create an owned `BufEncoder` prevented returning it
from functions which need to allocate the buffer on stack. Such is the
case in WIP serde via consensus serialzation.
This change refactors `OutBytes` to be unsized, adds an `AsOutBytes`
trait and uses that one instead of `Into` to perform the conversion.
b7eea6cb26 Update docs on rustfmt (Tobin C. Harding)
Pull request description:
We have introduced `rustfmt` but forgot to update the docs section about
it. Since a large portion of the codebase is currently ignored by our
`rustfmt` configuration, point out that `rusntfmt` is work in progress.
ACKs for top commit:
apoelstra:
ACK b7eea6cb26
Kixunil:
ACK b7eea6cb26
Tree-SHA512: c3a01e38e9787f7554847c657de8d2aeb512f237f68e97973ffa04e92decd282126abf16edb369c13dbeb9eed3587e20dbfb3ab77e06551a006a7aa2c70a71ad
4d54b161e7 Fix serde versions (Martin Habovstiak)
Pull request description:
`Cargo.toml` claimed that this crate works with very old versions of `serde` which wasn't the case. This commit changes the versions to minimal known-to-work values.
Adding `trivial` tag because the text change is trivial even though figuring it out wasn't. :)
If you manually decrease any of the versions by 1 (in both `Cargo.toml` and `Cargo.lock`) the build should fail.
ACKs for top commit:
apoelstra:
ACK 4d54b161e7
tcharding:
utACK 4d54b161e7
Tree-SHA512: 81ff3e9612aae4f3279d4dcb76ef4942d5047e7ddff84741e0ca269ee69a7631d062f6b5e6e43094e5d5f3e0b8799ddc224b716f630c9f0161185ed81fa0e96a
This adds a build script that automatically activates the cfg if the
version is sufficient. We don't make API changes here, just improve
debugging by enabling `track_caller`.
040b14ef1a Implement basic support for fast hex encoding (Martin Habovstiak)
Pull request description:
There's a `hex` module in `bitcoin_hashes` which is a bit out of place and not very fast - it passes each *digit* through dynamic dispatch not only adding overhead but also not allowing `String` to make better allocations.
This change adds basic support for hex encoding using a stack-allocated buffer which minimizes the overhead of dynamic dispatch. It also provides a new `DisplayHex` trait designed to replace `ToHex` found in `bitcoin_hashes`.
ACKs for top commit:
apoelstra:
ACK 040b14ef1a
tcharding:
ACK 040b14ef1a
Tree-SHA512: ffb2f46d5e0aa97c73a14067bdde92b7577651de1daee89fcb38b6e464b78a6ce40d39ac6bd573a9eec7274aa4e112ebc537604dca2c0d38da7d647cb548482a
Currently in one of the rustdoc examples showing lock satisfaction we
use two locks with the same value, this obfuscates which lock is doing
the satisfying and which lock is being satisfied.
Increment the value in one of the locks so it is obvious which lock is
which.
There's a `hex` module in `bitcoin_hashes` which is a bit out of place
and not very fast - it passes each *digit* through dynamic dispatch not
only adding overhead but also not allowing `String` to make better
allocations.
This change adds basic support for hex encoding using a stack-allocated
buffer which minimizes the overhead of dynamic dispatch. It also
provides a new `DisplayHex` trait designed to replace `ToHex` found in
`bitcoin_hashes`.
`Cargo.toml` claimed that this crate works with very old versions of
`serde` which wasn't the case. This commit changes the versions to
minimal known-to-work values.
834bbf461f Introduce bitcoin-internals crate (Tobin C. Harding)
12c5fb042e Refactor import statements (Tobin C. Harding)
022730bd8d Add a workspace to the top level directory. (Tobin C. Harding)
a59e0544ec Remove trailing whitespace (Tobin C. Harding)
Pull request description:
Add a new crate `bitcoin-internals` to be used for internal code needed by multiple soon-to-be-created crates. Add the `write_err` macro to `bitcoin-internals`, nothing else.
### Notes
This PR uses a `path` dependency which means `rust-bitcoin` cannot be released in its current state, will need to be changed once we release the `bitcoin-internals` crate on `crates.io`.
Please note also the testing strategy imposed by this PR - each new crate to have its own `contrib/test.sh` run from the top level `test.sh`.
Please excuse the mid-series import statement refactor, I had a moment of weakness and got uncontrollably triggered by the messy imports in one file, I managed to keep myself together on the others :)
ACKs for top commit:
apoelstra:
ACK 834bbf461f
Kixunil:
ACK 834bbf461f
Tree-SHA512: 327cced077b6d8bf6b4407a930ff8c0a6a8488256453cab168fc0d172ebb03976f8e64ff26a9b178f856c2ffa4342b5ef1a2c6eabcadf9bfa3cf787157a1d1ec
Add a new crate `bitcoin-internals` to be used for internal code needed
by multiple soon-to-be-created crates.
Add the `write_err` macro to `bitcoin-internals`, nothing else.
This patch uses a `path` dependency which means `rust-bitcoin` cannot be
released in its current state, will need to be changed once we release
the `bitcoin-internals` crate on `crates.io`.
Create a directory `bitcoin` and move into it the following as is with
no code changes:
- src
- Cargo.toml
- contrib
- test_data
- examples
Then do:
- Add a workspace to the repository root directory.
- Add the newly created `bitcoin` crate to the workspace.
- Exclude `fuzz` and `embedded` crates from the workspace.
- Add a contrib/test.sh script that runs contrib/test.sh in each
sub-crate
- Fix the bitcoin/contrib/test.sh script
d8116de56a Use Borrow trait bound for bip158 Iterator::Item (Tobin C. Harding)
Pull request description:
Currently the generic iterators in `bip158` return items of type slice reference. We can make the code cleaner (no explicit lifetimes) and more general if the generic iterator argument iterates over `Borrow<[u8]>`
ACKs for top commit:
Kixunil:
ACK d8116de56a
apoelstra:
ACK d8116de56a
Tree-SHA512: ce8111130d70a8c6f8d97f2c3f6c07bffd534461fdb599185129fc3cc4fc6c209e3187444082e47b7b55e4ec71fd8652fb2b86d2037573b99560ffd094175dc1
a9d1e08833 Add a comment to delineate test_macros re-export (Tobin C. Harding)
Pull request description:
The formatter removes an empty line that is clearly better to have. Add a rustdoc comment to at least delineate the line a bit.
ACKs for top commit:
Kixunil:
ACK a9d1e08833
apoelstra:
ACK a9d1e08833
Tree-SHA512: 0d962200a9d359825f3db50c2fcf3cf254f4f2ae55148d4a6524062bad8536160ae5f17256d4a86d746e66e5892e969b8efc14372f046ab7b4ce2950ce4bd25b
Currently the generic iterators in `bip158` return items of type slice
reference. We can make the code cleaner (no explicit lifetimes) and more
general if the generic iterator argument iterates over `Borrow<[u8]>`.
Note, includes adding `Borrow` to `crate::prelude`.
b9a512330a Simplify `PublicKey` serialization code (Martin Habovstiak)
Pull request description:
There were a bunch of `if self.serialized` expressions repeated in the code. This change simplifies it by putting the `if` in a function that calls a closure with a slice containing the serialized bytes.
ACKs for top commit:
apoelstra:
ACK b9a512330a
sanket1729:
utACK b9a512330a. I don't have strong preferences on naming. `serialize_and_call` sounds better because so far I have only seen `with_*` used with constructors.
tcharding:
ACK b9a512330a
Tree-SHA512: 5d90a73f87fa36bd45a5765542ec24994d601d32ad6529fff5d5140ded2b86eb40e00bfba4955e94d2c468983e3e7683ca5f8ece7a205d9b9944334a19b06015
81f81caa04 implement `From<PublicKey>` for `PubkeyHash` (Noah Lanson)
Pull request description:
Part of #1245.
Do we also want a method on `PubkeyHash` or is the `From` impl sufficient?
ACKs for top commit:
tcharding:
ACK 81f81caa04
Kixunil:
ACK 81f81caa04
sanket1729:
ACK 81f81caa04
apoelstra:
ACK 81f81caa04
Tree-SHA512: 760ea2991de60fd72d0aae18d375024f001f331d8a57b1e6e39b5933efa3a5af379effecb89cde0defb960b25c997b417fc126248589953655fb83db8b8dd608
86218ad240 Use macro to implement `std::erorr::Error` (Martin Habovstiak)
Pull request description:
There was a bunch of manual implemntations that can be converted to
macro call. This commit replaces them except for enums because those are
currently not supported by the macro and we want to protect against
forgetting to handle newly added variants.
Depends on #1129 and is **not** urgent for the next release.
ACKs for top commit:
apoelstra:
ACK 86218ad240
tcharding:
ACK 86218ad240
Tree-SHA512: a4ca91e023d66b5ad9408004b201cfe4cea85efb5a6e7f2241367934a954659c9196561295a491d2b2ed15c1a69c0ffb17a297d710cec4ce1d0f1ec8c12492e6
57c4283220 Remove unused consensus encoding error variants (Martin Habovstiak)
Pull request description:
The error variants `UnexpectedNetworkMagic` and `UnknownNetworkMagic` were not only duplicated but also completely unused. This removes both of them.
ACKs for top commit:
tcharding:
ACK 57c4283220
apoelstra:
ACK 57c4283220
Tree-SHA512: 3e686f54dee038500712e6960aeb3a633d95a6302f782b9b756a8a32f6b9eff805a7b63f2190c57ed806d6dfd7585b2f335bd8d5c5bfbe2c1b27e4761fd96625
0e425acad3 Run cargo fmt (Tobin C. Harding)
Pull request description:
A few trivial formatting errors have snuck into the codebase.
Run `cargo +nightly fmt`
FTR we have an open PR to run formatter in CI: https://github.com/rust-bitcoin/rust-bitcoin/pull/1111
ACKs for top commit:
Kixunil:
ACK 0e425acad3
apoelstra:
ACK 0e425acad3
Tree-SHA512: b42ba57fab25baf6ed7cfb3c207389df09d192840c591178fb3f43f1f0ff3e9381e1416c976fa4da8104dd3c18a32f54338ed98b997a31aba5c37a1b6c261210
840520042a BIP 34: Check last minimal encoding of encoded heights in coinbase tx (sanket1729)
fd3e01d6e3 Fix minimality of reading/writing CScriptNums (sanket1729)
Pull request description:
See: https://github.com/bitcoin/bips/blob/master/bip-0034.mediawiki
The BIP was updated with commit: 08844fd6ef
Found while reviewing #1240
Also cleanly deals with negative heights as errors.
ACKs for top commit:
apoelstra:
ACK 840520042a
tcharding:
ACK 840520042a
Tree-SHA512: 85eb1fbb428fb600f864390c3ac2e8e62efc1fc4b03b842e384ddb4ed9273c66d2274d0de219f1c7a6bad7b02bd8723f8f257c864577614df674b44bfc02010a
There was a bunch of manual implemntations that can be converted to
macro call. This commit replaces them except for enums because those are
currently not supported by the macro and we want to protect against
forgetting to handle newly added variants.
Andrew Poelstra
Tobin C. Harding
Jeffrey Czyz
Noah
Martin Habovstiak
sanket1729
Noah Lanson