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.
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
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.
The `Params` struct is currently defined in the `consensus` module which
has become a collection of orthogonal consensus-ish things. We would
like to put things in more descriptive places.
The `Params` struct defines constants that are network specific so it
makes sense to put it in the `network` module. As soft proof of this
argument note in this patch how often the `Params` type is imported
along with the `Network` type.
API break:
The type is no longer available at `bitcoin::consensus::Params` but
rather is re-exported at `bitcoin::network::Params`.
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.
Now that the `define_extension_trait` can handle function parameters on
individual lines revert the manual formatting that was introduced in
PR #2955.
Refactor only, no logic changes.
The `define_extension_trait` macro currently does not allow for a
trailing comma in the function paramater list, this occurs when the
function paramaters are put on individual lines (eg by `rustfmt`).
Extend the macro to support function paramaters being on individual
lines. This should have been done originally instead of my manual
override of formatting - bad Tobin no biscuit.
2169b75bba Use lower case error messages (Jamil Lambert, PhD)
Pull request description:
Error messages should be lower case, except for proper nouns and variable names. These have all been changed.
~~They should also state what went wrong. Some expect error messages were positive, giving the correct behaviour or correct input. These have been changed so that they are now negative, i.e. saying what went wrong.~~
EDIT: After further discussion it was decided not to change the expect messages.
ACKs for top commit:
Kixunil:
ACK 2169b75bba
tcharding:
ACK 2169b75bba
Tree-SHA512: 92442c869e0141532425f6fca5195fd319b65026f68c4230a65ad70253565d98931b2b44ee202975c307280525c505147e272297dc81207312e40c43d007021c
64c31cfb97 Move locktimes and Sequence to primitives (Tobin C. Harding)
Pull request description:
The `absolute` and `relative` locktimes as well as the `Sequence` are all primitive bitcoin types.
Move the `Sequence`, and `locktime` stuff over to `primitives`.
There is nothing surprising here, the consensus encoding stuff stays in `bitcoin` and we re-export everything from `blockdata`.
Note please `Sequence` is no longer publicly available at `bitcoin::transaction::Sequence` but it is available at `bitcoin::Sequence`.
ACKs for top commit:
Kixunil:
ACK 64c31cfb97
apoelstra:
ACK 64c31cfb97
Tree-SHA512: 968aa595bfb53e8fcfa860dae797ec5381ed49c67326a8ef9494086ec65d737502dffe4b24143e159042d07c59c5109d50103029082f87e1c3c26671e975f1b3
The `absolute` and `relative` locktimes as well as the `Sequence` are
all primitive bitcoin types.
Move the `Sequence`, and `locktime` stuff over to `primitives`.
There is nothing surprising here, the consensus encoding stuff stays in
`bitcoin` and we re-export everything from `blockdata`.
164b72e07b Update Key documentation (Shing Him Ng)
Pull request description:
The documentation for `Key` was a bit confusing since `<key> := <keylen> <keytype> <keydata>` was right above the `key` field, when in reality the `key` field represents the key data. Moving the documentation that describes a `Key` as a whole to the struct itself and specifying that `key` represents key data will hopefully clear things up a bit
ACKs for top commit:
Kixunil:
ACK 164b72e07b
tcharding:
ACK 164b72e07b
Tree-SHA512: 33c2b24d3006a0ed85a5d96351e0a01c1365c8ea01472233b024de54941319cdbefa0126f8b9538385e8f33ba7e2e3895f0dc5b6a36a1c501c8b97ebbede6502
bcf6d2839e Introduce scriptPubkey extension traits (Tobin C. Harding)
ee333defa4 Remove path from ScriptBuf (Tobin C. Harding)
Pull request description:
Done in preparation for moving the script types to `primitives`.
Add three public traits, one each for the three `script` types: `Builder`, `ScriptBuf`, and `Script`.
ACKs for top commit:
Kixunil:
ACK bcf6d2839e
apoelstra:
ACK bcf6d2839e
Tree-SHA512: 0ad11e474ddf1183d0119e36454cb4fd18d49a68655d274df800c6ef20afa7f8d0fdecd415c02595ea67a011e3a842b7ccc23c2d58f92ed9acbdc7f277fbd217
Martin Habovstiak
Tobin C. Harding
merge-script
Fmt Bot
Jamil Lambert, PhD
Shing Him Ng