b8067da934 Parse MSRV minor version number from env (yancy)
Pull request description:
Parse MSRV minor version number from env
replaces: https://github.com/rust-bitcoin/rust-bitcoin/pull/3145
ACKs for top commit:
Kixunil:
ACK b8067da934
tcharding:
ACK b8067da934
apoelstra:
ACK b8067da934 successfully ran local tests
Tree-SHA512: 60b3898c9b2739ca4858218b21d5c136b91b6ef45b1dc8f798d97aac861c42d046f114efcefdfa79f514d6da392f715a715ee6b3f230eb459f34e554b48ccf0e
0857697665 Replace impl blocks with extension traits (Martin Habovstiak)
b99bdcfdd6 Format `Script` blocks (Martin Habovstiak)
b027edffe7 Wrap `Script` impl blocks in temporary modules (Martin Habovstiak)
5a461545c7 Separate private `Script` methods (Martin Habovstiak)
27adc09e9f Generalize fn params in `define_extension_trait` (Martin Habovstiak)
fcc3cb03f0 Support non-doc attrs in extension trait macro (Martin Habovstiak)
ca1735f24c Separate POD methods (Tobin C. Harding)
Pull request description:
This moves methods from `Script` to extension traits in steps that should be easy to follow.
Moving to `primitives` requires doing the same with `ScriptBuf` so I'm holding off until this approach gets concept ACK (or alternatively someone else can do it :))
Closes#3161
ACKs for top commit:
tcharding:
ACK 0857697665
apoelstra:
ACK 0857697665 successfully ran local tests
Tree-SHA512: 3768d879e36139cf971c1921d3236141cbe87d707fd4bab7852f6ed8857b7867fa4146dfe720bd54e3d8cc50ecdc93886a10254cf9a82246358253f0312ffb47
2ec901fd63 Move the CompactTarget type to primitives (Tobin C. Harding)
a00bd7cc4d Introduce CompactTargetExt trait (Tobin C. Harding)
100ce03643 Run cargo +nightly fmt (Tobin C. Harding)
9c4a629659 Wrap CompactTarget impl block in temporary module (Tobin C. Harding)
578143c09e Separate CompactTarget impl blocks (Tobin C. Harding)
22d5646f7b Stop using CompactTarget inner field (Tobin C. Harding)
244d7dbe6c Remove generic test impl (Tobin C. Harding)
3d85ee3a02 primitives: Fix alloc feature (Tobin C. Harding)
Pull request description:
Done in preparation for moving `BlockHash` and `block::Header` to `primitives`.
- Patch 1 introduces an extension trait using `define_extension_trait!`
- Patch 2 is the trivial copy and past to move the type to `primitives`
This one shouldn't be to arduous to review, thanks.
ACKs for top commit:
Kixunil:
ACK 2ec901fd63
apoelstra:
ACK 2ec901fd63 successfully ran local tests
Tree-SHA512: b0e4f1af0b268e249a056cae71d7cafd1b025c4a079e5393ce80cd0b9c9bb6d2c6306531dc6786d986ff8a094b61866a86285b20d54037ef1395d127876bfd9c
In preparation to move script types to `primitives` we replace impl
block with extension traits by replacing the temporary modules with
`define_extension_trait`.
`rustfmt` is unable to format macro calls so instead we wrap the impl
blocks in modules to enable formatting in the next commit. We need to
change the visibility of the methods but that's OK since they're
internal.
The macro was trying to "parse" the parameters of functions defined in
extension trait. This was not needed and it was causing problems around
the `self` parameter. In this commit we change the macro to just pass
the parameters through.
Potentially the whole `pow` module will move to `primitives` but this
is not possible easily right now. However, we would like to be able to
move the `BlockHash` and `block::Header` types over to `primitives`
and doing so requires the `CompactTarget` to be there.
Move the `CompactTarget` type to `primitives` and re-export it from the
`primitives` crate root.
Note also, we re-export the type publicly from `bitcoin::pow`.
In preparation for moving the `CompactTarget` type to `primitives` stop
using the inner field in code that will stay behind in the
`bitcoin::pow` module.
In preparation for moving the `CompactTarget` to `primitives` remove the
generic `Into` impl and explicitly implement for just the `From` impls
that the `pow` unit tests use.
Test code only.
The `define_extension_trait` macro originally didn't support `#[inline]`
or other attributes for simplicity. We still want them so this commit
adds basic support for it. It adds the `doc` attributes to trait
*definition* only and adds all other attributes to the *implementation*
only. This should support `#[inline]` and other attributes. The downside
is it doesn't support adding non-doc attributes to trait *definition*
but I can't think of any relevant ones that we would want and we can
find a solution later if we do.
34e8212594 Replace &self with self: &Self (Tobin C. Harding)
Pull request description:
`foo(&self)` is syntax sugar for `foo(self: &Self)`.
The `define_extension_trait` is currently large, ugly, and not that expressive. If we use `self: &Self` then the macro is greatly simplified.
(Also consuming version `self: Self`)
De-sugar only, no logic changes.
ACKs for top commit:
apoelstra:
ACK 34e8212594 successfully ran local tests; lol this looks so much better
Kixunil:
ACK 34e8212594
Tree-SHA512: 7ec81bee99ede328d73a661c9e683a774ba14404ceb89ecb06765bedddf04dc9721672775b9ad3a9e3356d510c76681848f24ce4392a59d53216d23e6a27d9ad
579b76b7cb Introduce ToU64 conversion trait (Tobin C. Harding)
Pull request description:
The idea for this was pulled out of Steven's work in #2133
We already explicitly do not support 16 bit machines.
Also, because Rust supports `u182`s one cannot infallibly convert from a `usize` to a `u64`. This is unergonomic and results in a ton of casts.
We can instead limit our code to running only on machines where `usize` is less that or equal to 64 bits then the infallible conversion is possible.
Since 128 bit machines are not a thing yet this does not in reality introduce any limitations on the library.
Add a "private" trait to the `internals` crate to do infallible conversion to a `u64` from `usize`.
Implement it for all unsigned integers smaller than `u64` as well so we have the option to use the trait instead of `u32::from(foo)`.
ACKs for top commit:
Kixunil:
ACK 579b76b7cb
apoelstra:
ACK 579b76b7cb successfully ran local tests
Tree-SHA512: 2eaddfff995987a346e052386c6dfef3510e4732e674e3a2cfab60ee391b4cce1bf7ba4fb2dfd4926f8203d7251eea2198ccb61f0b40332e624c88fda4fa7f48
191897f9ea Manually format (Tobin C. Harding)
Pull request description:
Run `rustfmt` and manually fix the places where comments are moved to the wrong place.
ACKs for top commit:
Kixunil:
ACK 191897f9ea
apoelstra:
ACK 191897f9ea successfully ran local tests
Tree-SHA512: f977ff373d1d410012734208c090bfcd8f9dbda414d0b19400acf8f552df481b4a2bc20d77c61538895a6fb66197be13cbdadf74956d67fd4d055b99ba8ab356
We already explicitly do not support 16 bit machines.
Also, because Rust supports `u182`s one cannot infallibly convert from a
`usize` to a `u64`. This is unergonomic and results in a ton of casts.
We can instead limit our code to running only on machines where `usize`
is less that or equal to 64 bits then the infallible conversion is
possible.
Since 128 bit machines are not a thing yet this does not in reality
introduce any limitations on the library.
Add a "private" trait to the `internals` crate to do infallible
conversion to a `u64` from `usize`.
Implement it for all unsigned integers smaller than `u64` as well so
we have the option to use the trait instead of `u32::from(foo)`.
`foo(&self)` is syntax sugar for `foo(self: &Self)`.
The `define_extension_trait` is currently large, ugly, and not that
expressive. If we use `self: &Self` then the macro is greatly
simplified.
De-sugar only, no logic changes.
6836de9ee6 Remove catch all pattern (Tobin C. Harding)
Pull request description:
The `PushBytes` type enforces len is less than 0x100000000 so we do not need to panic in a catch all pattern after explicitly matching against less than 0x100000000.
Refactor only because of the invariant on `PushBytes` - no logic changes.
ACKs for top commit:
apoelstra:
ACK 6836de9ee6 successfully ran local tests
Kixunil:
ACK 6836de9ee6
Tree-SHA512: a7cdb31683a8c00eecbdd0879886bec48133f9029f899b5279f1f5294ef40320592db196bfcafdeef4507636fc785a7ab87879b25b6d1b4905ae573b545f1ff4
The `PushBytes` type enforces len is less than 0x100000000 so we do
not need to panic in a catch all pattern after explicitly matching
against less than 0x100000000.
Refactor only because of the invariant on `PushBytes` - no logic
changes.
84df3438ca Fix markdown list items (Tobin C. Harding)
0a45c68cf8 Introduce helper function name policy (Tobin C. Harding)
Pull request description:
As much as it hurts the C hacker inside me we have settled on using `_internal` to mark private function names that clash with a public function of the same name.
Introduce a policy section and rename one instance, I did not grep the codebase looking for other violations.
This came up because I had to look at what `_inner` implied when reading the function name somewhere else.
ACKs for top commit:
storopoli:
ACK 84df3438ca
apoelstra:
ACK 84df3438ca successfully ran local tests; sgtm. should probably update rust-miniscript which uses a `real_` prefix
Tree-SHA512: aece73fac54c4f34bdba72c08721586eb6d76dc9191442bbca43301f2af3d3b3e3217383f1ad20752fa7654e7f5d09927d1a4b72602aa064e76d550b5d6e81bd
As much as it hurts the C hacker inside me we have settled on using
`_internal` to mark private function names that clash with a public
function of the same name.
Introduce a policy section and rename one instance, I did not grep the
codebase looking for other violations.
This came up because I had to look at what `_inner` implied when reading
the function name somewhere else.
In an effort to reduce the cognitive load of reading code we are
removing casts unless they are useful or obvious.
Move the cast onto the call to `min` and comment it for good measure.
This allows us to call infallible `from` for conversion when needed.
Refactor only, no logic changes.
In an effort to remove unnecessary casts use `u128::from` to convert
from `u64`s. Leave the cast to `u64` in there because it is right after
a shift right and is brain-dead obvious.
A single trait bound can be expressed using the `impl` style. This is a
breaking change because callers can no longer use turbofish. In this
case that probably does not matter because users are likely just passing
an integer in and letting the compiler infer the type.
Done in preparation for moving logic into an extension trait so that the
functions can be parsed by the `define_extension_trait` macro.
ref: https://doc.rust-lang.org/reference/types/impl-trait.html
c72069e921 Bump MSRV to 1.63 (Martin Habovstiak)
Pull request description:
The version 1.63 satisfies our requirements for MSRV and provides significant benefits so this commit bumps it. This commit also starts using some advantages of the new MSRV, namely namespaced features, weak dependencies and the ability to use trait bounds in `const` context.
This however does not yet migrade the `rand-std` feature because that requires a release of `secp256k1` with the same kind of change - bumping MSRV to 1.63 and removing `rand-std` in favor of weak dependency. (Accompanying PR to secp256k1: https://github.com/rust-bitcoin/rust-secp256k1/pull/709 )
Suggested plan:
* merge both PRs
* at some point release `hashes` and `secp256k`
* remove `rand-std` from `bitcoin`
* release the rest of the crates
ACKs for top commit:
apoelstra:
ACK c72069e921
tcharding:
ACK c72069e921
Tree-SHA512: 0b301ef8145f01967318d3ed1c738d33e6cf9e44f835f3762122b460a536f926916dbd6ea39d6f80b4f95402cd845e924401e75427dbb0731ca5b12b4fa6915e
feef34fdea Make ScriptBuf::p2wpkh_script_code stand alone (Tobin C. Harding)
Pull request description:
We would like to move the `Script` type to `primitives` without moving any key stuff, including pubkey hashes. We may later, before releasing `primitives`, move the `WPubkeyHash` at which time this patch can be reverted or re-implemented on `ScriptBuf`.
The `ScriptBuf::p2wpkh_script_code` function does not take `self` as a parameter but it does return `Self` - this can trivially be made into a standalone function.
Make `ScriptBuf::p2wpkh_script_code` a standalone function.
ACKs for top commit:
Kixunil:
ACK feef34fdea
apoelstra:
ACK feef34fdea
Tree-SHA512: 9de43bb274480b85d328ddd4cb6d79b501a525837307ca399dea1c3d39e9a7c0d8719dbb0e5d852629351d2b1fe09ce0244df82c53ff9ef2342be85a71887329
The version 1.63 satisfies our requirements for MSRV and provides
significant benefits so this commit bumps it. This commit also starts
using some advantages of the new MSRV, namely namespaced features, weak
dependencies and the ability to use trait bounds in `const` context.
This however does not yet migrade the `rand-std` feature because that
requires a release of `secp256k1` with the same kind of change - bumping
MSRV to 1.63 and removing `rand-std` in favor of weak dependency.
29b213daca Move validation module to consensus_validation (Tobin C. Harding)
Pull request description:
The `consensus` module is currently doing two things, validation and encoding. These two things are orthogonal.
Move the `consensus::validation` module to `consensus_validation`. Remove the function re-exports from `consensus`.
This was originally discussed here: https://github.com/rust-bitcoin/rust-bitcoin/issues/2779
ACKs for top commit:
Kixunil:
ACK 29b213daca
apoelstra:
ACK 29b213daca
Tree-SHA512: 3bd0e43c220b0d89a47e9df0e0c92b776ccc65f5f60d57f413db834acc8e86269379bc9fdd688f8c4f0138db22f8eb8983770afa2d7d53d51acf063f2302121c
We would like to move the `Script` type to `primitives` without moving
any key stuff, including pubkey hashes. We may later, before releasing
`primitives`, move the `WPubkeyHash` at which time this patch can be
reverted or re-implemented on `ScriptBuf`.
The `ScriptBuf::p2wpkh_script_code` function does not take `self` as a
parameter but it does return `Self` - this can trivially be made into a
standalone function.
Make `ScriptBuf::p2wpkh_script_code` a standalone function.
3b15ef6d27 Fix rustdocs in `blockdata` (Jamil Lambert, PhD)
Pull request description:
Following up on the [comment](https://github.com/rust-bitcoin/rust-bitcoin/pull/2646#discussion_r1548848611) in #2646 the rustdocs formatting was fixed in `blockdata`.
ACKs for top commit:
Kixunil:
ACK 3b15ef6d27
tcharding:
ACK 3b15ef6d27
Tree-SHA512: 509aa2b8ae559f5a7f8230c016c0866f468cc147773238e226b9a975784d7a424c41a5d966ddcf9b3f8f8d4fe197a4f272b5777c61f3cc53051803abb520b95e
ab581a90f8 Remove re-export of ParseIntError (Tobin C. Harding)
Pull request description:
In d242125 I claimed that `ParseIntError` was somehow special, I no longer thing this is the case. As we pin down the re-export policy (for errors and other types) it is hard if we have one non-typical re-export.
We have https://github.com/rust-bitcoin/rust-bitcoin/issues/3068 to discuss the policy, for now just remove the unusual re-export.
ACKs for top commit:
Kixunil:
ACK ab581a90f8
shinghim:
ACK ab581a90f8
apoelstra:
ACK ab581a90f8
Tree-SHA512: 5ac4123aeb27c8cee78e5760f21e70be8035d526ba7e14e72759cba27f98b51cc2cba9b2bf0eeb99e0f6b7210ec4a750986bb6c5dc0725ed892730fdec8a7e06
54c30556a2 Move params to network module (Tobin C. Harding)
045a661ebe Create network directory (Tobin C. Harding)
Pull request description:
Discussed in #2779. Patch one moves `network.rs` to `network/mod.rs`, and patch 2 moves the `params` module over there.
ACKs for top commit:
apoelstra:
ACK 54c30556a2 Yeah, this seems like a good place for it
Kixunil:
ACK 54c30556a2
Tree-SHA512: 134813419db21323d303d465b12fcbf37fd61dc1baf3305e156d324eafd822379e63dede02877ee99dce41540193a29e6e13acd13f9121f3e2fe11096524aa5e
The `consensus` module is currently doing two things, validation and
encoding. These two things are orthogonal.
Move the `consensus::validation` module to `consensus_validation`.
Remove the function re-exports from `consensus`.
386ad93253 Manually format function parameters (Tobin C. Harding)
871f4398b9 Add optional comma to function parameter list (Tobin C. Harding)
Pull request description:
During #2955 I was lazy and did not think through why the macro didn't handle function parameters on individual lines, I just manually re-formatted all function calls onto a single LOC. This was a bit slack of me.
- Patch 1: Fix the macro.
- Patch 2: Revert the manual formatting.
ACKs for top commit:
jamillambert:
ACK 386ad93253
Kixunil:
ACK 386ad93253
apoelstra:
ACK 386ad93253
Tree-SHA512: 577154668a4cd0bad225a5b306193a5cd3d38d275dd5df22b4a29737e0d2c910c695da33c0ed77265f6c5d876c162316e444f8eca80f9c30788d110d989493e3
beea3c1e5d Remove public error re-export (Tobin C. Harding)
Pull request description:
We do not have a policy to re-export things from other modules just because they are in the public API - I don't see any other reason to re-export this error, users should go to the `validation` module directly to get the error type.
Raising this trivial change as a separate PR so that we can really pin down our re-export policy. Please review the policy implications as well as the code change.
Note please that this change was introduced in 7d695f6b4 by me, and buried in a PR that did not mention the change. This was wrong, as in the code change was wrong and also the patching method was wrong.
ACKs for top commit:
Kixunil:
ACK beea3c1e5d
Tree-SHA512: 5fc072f3fb8a727f30751211c6bc85dc268d413ee62937c714bdf9f47405dfdbc93cfff3df76c201493c39f49d5d315907fc9e7e4fa0d927652c01038815fdc5
merge-script
Martin Habovstiak
Tobin C. Harding
yancy
Antoni Spaanderman
Fmt Bot
Jamil Lambert, PhD