a184066660 Move import inside feature gate (Tobin C. Harding)
Pull request description:
The `String` type is only used if the `serde` feature is enabled, move the import statement inside the already feature gated block.
ACKs for top commit:
apoelstra:
ACK a184066660 successfully ran local tests; sure
Kixunil:
ACK a184066660
Tree-SHA512: 688adb1e4489b0242856ac36ed9cf3503b147b84954cf1e6fd34b5e708ed6c1dd92da91e739243face1c1b4e4c8b19ce88e215117bef2a7af2e732859a6e108a
dae42bef9d do not enable bitcoin-io by default (Antoni Spaanderman)
a14cdaf859 don't enable std by default when testing (Antoni Spaanderman)
e83830dcfc use slice instead of array to not have to hardcode the length (Antoni Spaanderman)
55749d6f61 use `hash.to_byte_array` to check equality with `test.output` (Antoni Spaanderman)
969864e3b0 use fixed size array if possible, otherwise `&'static [u8]` (Antoni Spaanderman)
28ccf70fa6 remove unnecesarry borrow operator (`&`) (Antoni Spaanderman)
fa3a3afd02 remove unnecessary slicing (Antoni Spaanderman)
22e42ab86c fix test code being unnecessarily feature gated (Antoni Spaanderman)
Pull request description:
- remove 2 unnecessary cfg attributes from tests left over from #3167 (it made them not dependent on `alloc` anymore)
- simplify assertion logic by removing unnecessary conversions before comparing
- make tests `no_std` compatible by adding imports to alloc or std
- feature gate tests behind the `alloc` feature if they use anything from the alloc crate (like the `format!` macro)
- `schemars` feature enables `alloc` because (for example) its trait wants implementations to return `String`
- fix `bitcoin-io` always enabling when `std` is enabled (only useful if people depend on `hashes` only, `bitcoin` depends on `bitcoin-io` already)
ACKs for top commit:
tcharding:
ACK dae42bef9d
Kixunil:
ACK dae42bef9d
apoelstra:
ACK dae42bef9d successfully ran local tests
Tree-SHA512: 622fd4963ef21530a98af89bcfc71abe8723aac12d363ab88d9bd30dcf2f75392711bec10e2901fab5f1a30e11897d1aae36e22892738aa1e5670166f91fddd4
A new nightly version (`nightly-2024-08-28`) introduces a few warnings
because of our rustdocs. These are valid warnings and should be fixed,
thanks `clippy` team.
(The `bip152` change is a bit sloppy, open to suggestions.)
cf129ad314 fix: re-implement (de)serialization from/to readers/writers (elsirion)
Pull request description:
Fixes#3250.
The serialization is less than ideal and still allocates a lot. I can understand not wanting to (ab)use the consensus encoding traits, but they have a pretty good interface, copying it and creating some `EncodePsbt` and `DecodePsbt` traits with similar interfaces would have been nice imo.
ACKs for top commit:
Kixunil:
ACK cf129ad314
apoelstra:
ACK cf129ad314 successfully ran local tests; LGTM -- I believe this is non-breaking, as does cargo-semver-checks, so we can backport this to 0.32
Tree-SHA512: d7f218164d772db3a9fb4436953c3b5fd3677b92078d0843233197629df7d852d80615a3ff38c5b70771381ba1aeb30defdc98ee63653e570bb75dc553400cad
a76d76eca1 Change `T::from_str(s)` to `s.parse::<T>()` (Jamil Lambert, PhD)
Pull request description:
As mentioned in issue #3234 `s.parse::<T>()` is more idiomatic and produces more helpful error messages.
This has been changed in the main codebase, not including examples, rustdocs, and in the `test` modules.
`use std::str::FromStr;` has been removed where this change makes it unnecessary.
To close the issue it may also need to be changed in the examples and the `test` modules and `contributing.md` updated.
ACKs for top commit:
Kixunil:
ACK a76d76eca1
storopoli:
ACK a76d76eca1
tcharding:
ACK a76d76eca1
Tree-SHA512: 18be389579b5c2d0ba567dfb02b9c8d71c108a749ec5168c60a50e7af9d7f498e364c8f4c0ce1698d960e7d146486899cd10214ebc9c96708ed158a04dfff425
`s.parse` is more idiomatic and produces more helpful error messages.
This has been changed repo wide in the main codebase, not including
examples, rustdocs, and in the test module.
`use std::str::FromStr;` has been removed where this change makes
it unnecessary.
The two `TxOut` fields are public and there are no construtors or
getters to move, only the associated const `NULL`.
Add a tmp module around the big impl block so we can trick the formatter
into indenting before we add the extension trait.
3e034d5ede Add Arbitrary dependency (yancy)
Pull request description:
Adds an example draft showing what is needed to use Arbitrary for coin selection.
Shot out to how nice Arbitrary is for fuzzing a target by taking unstructured randomness and creating structured rust-bitcoin types for fuzzing. Is there a way we could add this to rust-bitcoin for structuring the fuzz data needed?
This is then the example to fuzz test a SRD algo (after applying this PR to rust-bitcoin) using rust-bitcoin types :)
```
#![no_main]
use arbitrary::Arbitrary;
use bitcoin::{Amount, FeeRate};
use bitcoin_coin_selection::{select_coins_srd, WeightedUtxo};
use libfuzzer_sys::fuzz_target;
use rand::thread_rng;
#[derive(Arbitrary, Debug)]
pub struct Params {
target: Amount,
fee_rate: FeeRate,
weighted_utxos: Vec<WeightedUtxo>,
}
fuzz_target!(|params: Params| {
let Params { target: t, fee_rate: f, weighted_utxos: wu } = params;
select_coins_srd(t, f, &wu, &mut thread_rng());
});
```
ACKs for top commit:
tcharding:
ACK 3e034d5ede
Kixunil:
ACK 3e034d5ede
apoelstra:
ACK 3e034d5ede successfully ran local tests
Tree-SHA512: accd565815de3b37730d2ff12a24fcfc84e52ad357e5c940b1500a1e0bb17f4ff5fd6e52d31e8e96bb5290ee4fa050cfd2a9bbd6bbae13fc378f43093b64177f
9fb5edb39e ecdsa: Improve error types (Tobin C. Harding)
Pull request description:
There are a couple of issues around the ECDSA signature decoding / parsing code. We have duplicate code in `from_str` and `from_slice` and both use the same error type even though it is impossible to get a hex error in `from_slice`.
Create two errors:
- A `DecodeError` returned by `from_slice`
- A `FromStrError` that has a decode variant and a hex variant
Call through to `from_slice` after parsing hex into a byte vector.
Removes an instance of `unreachable!`.
Fix: #1193
ACKs for top commit:
Kixunil:
ACK 9fb5edb39e
apoelstra:
ACK 9fb5edb39e successfully ran local tests
Tree-SHA512: 3b3ae31887d603f1739d261b491b99f7847987f94dbbfefb9aa84d4250736eba2d007d28746bbb064946d3055e4cca01510677bf2cdbb11bbf83d7388dbd2620
c427d8b213 bitcoin: Compile time assert on index size (Tobin C. Harding)
49a6acc1a0 internals: Remove double parenthesis in const_assert (Tobin C. Harding)
2300b285ef units: Remove compile time pointer width check (Tobin C. Harding)
Pull request description:
3 patches in preparation for other size related work, this PR does not touch the `ToU64` issue which will be handled separately.
- Patch 1: Don't check pointer width in `units` because its not consensus code
- Patch 2: Modify internal macro `const_assert`
- Patch 3: Use index size to enforce not building on a 16 bit machine
ACKs for top commit:
Kixunil:
ACK c427d8b213 though I think the last commit was kinda a waste of time and it should have been adding the trait instead or leave it for later.
apoelstra:
ACK c427d8b213 successfully ran local tests; unsure if we want to merg this or wait for #3215
Tree-SHA512: 823df5b6a5af3265bce2422c00d287f45816faeb5f965685650ac974a1bd441cf548e25ac2962591732ff221bee91a55703da936382eb166c014ca5d4129edf8
a2be82c0c9 Use TBD in deprecated attribute (Tobin C. Harding)
Pull request description:
Our `release` job checks for 'TBD', I can't remember exactly why but I thought we introduced `0.0.0-NEXT-RELEASE` because CI was failing when we used TBD - clearly this is not the case now because we have a bunch of `TBD`s in the code base.
Change all the instances of `0.0.0-NEXT-RELEASE` to be `TBD`.
ACKs for top commit:
Kixunil:
ACK a2be82c0c9
apoelstra:
ACK a2be82c0c9 successfully ran local tests
Tree-SHA512: b383cc4095484291a7b4dca593ad5e017e3a9de9bfae9d6e9447ae36da32aa1c0d1fd593f49fd52c04db5ca5cdbaae8b30a772f792df13542f0a157a86295746
96e0e720fd feat(bip158): compute canonical filter hash (Rob N)
Pull request description:
From [BIP-157](https://github.com/bitcoin/bips/blob/master/bip-0157.mediawiki#filter-headers)
> The canonical hash of a block filter is the double-SHA256 of the serialized filter.
If a user forgets the "double" in double-SHA256 they will be computing a nonsensical filter hash when this is easily handled by the API.
ACKs for top commit:
Kixunil:
ACK 96e0e720fd
tcharding:
ACK 96e0e720fd
Tree-SHA512: 5fc0b1632e2327adacbd0ab56b4cd7edce0774f8be2f819782519c51b9691a748f4b3c01887f3bf1146dee49a35f9dfac833f53ae69ee7a53858bd2cedcec01a
There are a couple of issues around the ECDSA signature decoding /
parsing code. We have duplicate code in `from_str` and `from_slice`
and both use the same error type even though it is impossible to get a
hex error in `from_slice`.
Create two errors:
- A `DecodeError` returned by `from_slice`
- A `ParseSignatureError` that has a decode variant and a hex variant
Call through to `from_slice` after parsing hex into a byte vector.
Removes an instance of `unreachable!`.
Fix: #1193
Our `release` job checks for 'TBD', I can't remember exactly why but I
thought we introduced `0.0.0-NEXT-RELEASE` because CI was failing when
we used TBD - clearly this is not the case now because we have a bunch
of `TBD`s in the code base.
Change all the instances of `0.0.0-NEXT-RELEASE` to be `TBD`.
Recently we made it so that wrapper types created with `hash_newtype`
were not general purpose hash types i.e., one could not easily hash
arbitrary data into them. We would like to do the same for tagged
wrapped hash types.
In `hashes` do:
- Create a new macro `sha256t_tag` that does just the tag/engine stuff
out of the `sha256t_hash_newtype` macro.
- Deprecate the `sha256t_hash_newtype` macro.
In `bitcoin` do:
- Use a combination of `sha256t_tag` and `hash_newtype` to create tagged
wrapped hash types.
Note that we do not add private helper functions `engine` and
`from_engine` to the tagged wrapper types as we do for legacy/segwit in
`sighash`. Can be done later if wanted/needed.
2bb90b8203 Introduce two extensions traits for ScriptBuf (Tobin C. Harding)
ae0a5bd64a Run cargo fmt (Tobin C. Harding)
3fdc574851 Add temporary script buf modules (Tobin C. Harding)
4ff5d6886b Add private ScriptBufAsVec type (Tobin C. Harding)
c81fb93359 Make push_slice_no_opt pub(crate) (Tobin C. Harding)
1001a33f19 Add second ScriptBuf impl block (Tobin C. Harding)
3625d74e8b Make pub in crate functions pub crate (Tobin C. Harding)
b368384317 Separate ScriptBuf POD methods (Tobin C. Harding)
Pull request description:
Similar to #3155 but for `ScriptBuf`, however it is a little more involved.
Note:
- the change to use `impl` syntax (and addition of #3179)
- mad trickery of `ScriptBufAsVec` (props to Kix)
- widening of scope of private functions
Onward and upward!
ACKs for top commit:
Kixunil:
ACK 2bb90b8203
apoelstra:
ACK 2bb90b8203 successfully ran local tests
Tree-SHA512: 7209d8dc436e52b23e1dbfd9db8432df225ebdb701f465e4d1b55328e22988c98a0f28efdf2a8b3edbafc754354d718ab36bd2f5b1621d12e061b2dadaf49a05
In preparation for moving the `ScritpBuf` type to `primitives` add a
public and private extension trait for the functions we want to leave
here in `bitcoin`.
Note, includes a change to the `difine_extension_trait` metavariable
used on `$gent` from `ident` to `path` to support the generic
`AsRef<PushBytes>`.
Add a private type that allows us to mutate the inner vector of a
`ScriptBuf` only using public functions and never touching the inner
field.
Done in preparation for moving the `ScriptBuf` to `primitives`.
Mad hackery by Kix!
In preparation for adding script buf extension make the
`push_slice_no_opt` have the same scope as the other private functions,
this will be the scope of the private extension trait.
In preparation for adding a private extension trait change the scope to
`pub(crate)` because the more specific `pub(in ...)` is not currently
supported by our `define_extension_trait` macro.
In preparation for moving the `ScriptBuf` as a plain old datatype to
`primitives`; separate the POD methods into their own impl block.
Refactor only, no logic changes.
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.
Currently we enforce that our code only runs on machines with a
certain pointer width (32 or 64 by failing to compile if pointer size
width is 16). One of the underlying reasons is because of requirements
in consensus code in Bitcoin Core which requires containers with more
than 2^16 (65536) items [0].
We can better express our requirements by asserting on Rust's index
size (the `usize` type).
As a side benefit, there is active work [1] to make Rust support
architectures where pointer width != idex size. With this patch applied
`rust-bitcoin` will function correctly even if that work progresses.
- [0] https://github.com/rust-bitcoin/rust-bitcoin/pull/2929#discussion_r1659399813
- [1] https://github.com/rust-lang/rust/issues/65473
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
merge-script
Jiri Jakes
Tobin C. Harding
Jamil Lambert, PhD
elsirion
Antoni Spaanderman
Fmt Bot
yancy
Rob N
leichak
Martin Habovstiak
Shing Him Ng