051c358bcb Remove deprecated legacy numeric methods (Divyansh Gupta)
Pull request description:
As `rustc 1.79.0-nightly (9d79cd5f7 2024-04-05)` is released which solves the issue mentioned , but the release has deperacted legacy numeric methods.
Thus replaced `u16::max_value()` etc with `u32::MAX` & `core::u16` to directly `u16`.
fix#2639
ACKs for top commit:
tcharding:
ACK 051c358bcb
apoelstra:
ACK 051c358bcb thanks! I will remove an equivalent commit from my #2669
Tree-SHA512: c08c856f7f3b281417c29283351eac5e0f75cc1c8d23d9aae58d969219a327b2337fe57932053e53773ebb9dbec04254f90149266b6639a66c5c09f2ad1675ef
As `rustc 1.79.0-nightly (9d79cd5f7 2024-04-05)` is released which solves the issue mentioned , but the release has deperacted legacy numeric methods.
Thus replace `u16::max_value()` etc with `u32::MAX` & `core::u16` to directly `u16`.
fix#2639
When signing a Taproot input (in a PSBT) using a key path spend we
currently return the pubkey associated with key that signs. However it
is common to think of the internal key as being the one that signs even
though this is not technically true. We also have the internal key in
the PSBT so matching against it is less surprising.
When using the `Psbt` type to sign a Taproot input using a key path
spend return the internal key.
There is no need to panic if input index is out of bounds because we
have a function to check the validity of the `input_index` argument and
use it in other places already.
f6467ac98d Minimize usage of Network in public API (Tobin C. Harding)
3ec5eff56e Add Magic::from_params (Tobin C. Harding)
Pull request description:
Minimize usage of the `Network` enum in the public API.
See #2225 for context, and https://github.com/rust-bitcoin/rust-bitcoin/pull/1291#discussion_r1492993788 for an interpretation of that long discussion.
Close: #2169
ACKs for top commit:
sanket1729:
reACK f6467ac98d.
apoelstra:
ACK f6467ac98d
Tree-SHA512: f12ecd9578371b3162382a9181f7f982e4d0661915af3cfdc21516192cc4abb745e1ff452649a0862445e91232f74287f98eb7e9fc68ed1581ff1a97b7216b6a
A release or so ago we added `non_exhaustive` to the `Network` enum,
this turned out to make usage of the enum un-ergonomic for downstream
users. After much debate we decided that a way forward was to just
minimize the usage of the enum in the public API by instead use
`AsRef<Params>` so that downstream could define their own network enum
based on the networks they support.
Minimize usage of `Network` by using `AsRef<Params>` as a parameter type
instead. "minimize" because the `Network` still appears in some places.
Currently we are using `Self` (in backticks) in the docs to functions
defined by the `do_iml` macro, this is a bit lazy, we can do better than
that.
Use `doc` attribute and the `$ty` macro variable to construct the docs
to use the type name.
Use a code comment to document the calling restrictions of private
function `from_hex_internal`. (Code comment because comment is not well
formed as per convention in this codebase.)
Improve the rustdocs on the private `U256` type by doing:
- Remove link to self within constructors, just use backticks
- Use `U256` instead of `Self` or `self`
- Fix incorrect usage of `CompactTarget` [0]
[0] We knew this was wrong when we merged it but because the docs are
private we elected to do this follow up patch.
Currently `Magic` has per network consts but no way to dynamically get
the magic bytes for a network. Note also that we are currently trying to
reduce the usage of `Network` in the public API.
Add a public constructor to the `Magic` type that accepts a `Params`
parameter to determine the network to use.
The `pow` types implement `fmt::LowerHex` but do not implement hex
parsing.
Add inherent methods `from_hex` and `from_prefixed_hex` to the
`pow` types.
fd6fedc3ad Improve API for max target threshold calculation (Tobin C. Harding)
6e47d57744 Rename difficulty transition threshold functions (Tobin C. Harding)
4121c9a09f Rename Params::pow_limit to max_attainable_target (Tobin C. Harding)
f0f6d3f162 Take Params instead of Network in difficulty function (Tobin C. Harding)
104dee9376 Debug assert that target != zero in difficulty calc (Tobin C. Harding)
c1ba496a07 Document current behaviour of difficulty_float (Tobin C. Harding)
3d01146374 Allow needless-borrows-for-generic-args (Tobin C. Harding)
2a6821b426 Use link to CompactTarget in rustdoc (Tobin C. Harding)
Pull request description:
When computing the maximum difficulty transition threshold we forgot to check that the returned `Target` is not bigger than the maximum. This value is network specific so keep the original logic but with `_unchecked` on the function name.
This was noted in the discussion on #2161
ACKs for top commit:
apoelstra:
ACK fd6fedc3ad
sanket1729:
ACK fd6fedc3ad
Tree-SHA512: 520ee2a07edb251c84b5ce8b48ed6e5a5c1945126dc7bcdb5570e97101ec4a3dc63fa7992725194869e22b21ee4f5955579d5e2499fcb48167637fd1fb3ae74d
The maximum target threshold has a network dependant upper bound.
Currently we are not checking this bound. One complication is that there
is currently heated open debate around the `Network` type.
We can bypass the `Network` issue by using `AsRef<Params>` instead.
Add a function that does the checks based on the `Params` type as well
as an unchecked version.
These two functions calculate the min/max threshold transition which is
a _target_ not a "difficulty" number. Using "difficulty" in the function
name is unnecessarily confusing.
Rename and deprecate the functions.
The maximum "attainable" target is a `rust-bitcoin` thing, Core use max
unattainable.
Deprecated the `Params::pow_limit` field and add a new field
`max_attainable_target`.
The `Params` type is `non_exhaustive` so this is not an API breaking
change.
What we really want is the maximum target, but since this is a const in
`Params` use an `AsRef<Params>` argument in the `difficulty` functions.
Requires implementation of `AsRef<Params> for Params`.
The `difficulty` calculation requires dividing a target value by `self`.
Add an assertion that `self` is not zero to help devs debug this.
Note that this should never really be hit, but its possible there is a
bug somewhere causing the target to be set to zero - so this may help
debugging.
Also, add panics section to rustdocs.
This lint triggers when parsing a reference to a large struct as a
generic argument, which is wrong.
Allow it crate wide because [subjectively] this lint never warns for
anything useful.
d91cdd20bf docs: Document ordered feature (Tobin C. Harding)
3520f550f0 Implement ArbitraryOrd for relative::LockTime (Tobin C. Harding)
Pull request description:
TL;DR As we do for `absolute::LockTime` and for the same reasons; implement `ArbitraryOrd` for `relative::LockTime`.
locktimes do not have a semantic ordering if they differ (blocks, time) so we do not derive `Ord` however it is useful for downstream to be able to order structs that contain lock times. This is exactly what the `ArbitraryOrd` trait is for.
Fix: #2566
ACKs for top commit:
sanket1729:
ACK d91cdd20bf
apoelstra:
ACK d91cdd20bf
Tree-SHA512: 52ace9222e765dfa266d003b4aff3e93e35d1414c9fd579c4a4a36998d6d1b08bf6d4964a6f1c1d769068d65e47a882495daa4aacf254909a35dce8e01c99a9e
TL;DR As we do for `absolute::LockTime` and for the same reasons;
implement `ArbitraryOrd` for `relative::LockTime`.
locktimes do not have a semantic ordering if they differ (blocks, time)
so we do not derive `Ord` however it is useful for downstream to be able
to order structs that contain lock times. This is exactly what the
`ArbitraryOrd` trait is for.
Update the rustdocs in `relative` and mirror the docs changes in
`absolute`.
Fix: #2566
`require_network` is typically called as part of parsing, often in the
same line of code. Counter to our normal errors, it makes
`require_network` more ergonomic to use if we just return a `ParseError`
variant.
Close: #2507
Done in preparation for adding the `NetworkValidationError` as a variant
of `ParseError`.
Move the `NetworkValidationError` type to beneath `ParseError`.
Code move only, no other changes.
fd040f5e38 Replace TBD with 0.32.0 (Tobin C. Harding)
Pull request description:
We are gearing up for the 0.32.0 release; replace all instances of TBD with the version number of the upcoming release.
ACKs for top commit:
sanket1729:
ACK fd040f5e38
apoelstra:
ACK fd040f5e38
Tree-SHA512: fe73fd47a794557742f618b21434cd3cc18cde0e861216716723bfcc9135accf63590e1ea60bfeda066acec7312c8b9f1bf09e7454e7161ccaba5ebe60af66fd
af49841433 Hide base58::Error internals (Tobin C. Harding)
4f68e79da0 bitcoin: Stop using base58 errors (Tobin C. Harding)
669d5e8fc6 base58: Add InvalidCharacterError for decoding (Tobin C. Harding)
ec8609393b base58: Add error module (Tobin C. Harding)
42fabbab03 base58: Run the formatter (Tobin C. Harding)
Pull request description:
Improve the error code in the new `base58` crate.
ACKs for top commit:
apoelstra:
ACK af49841433
sanket1729:
ACK af49841433
Tree-SHA512: c05479f02a9a58c7c98fd5987e760288562372e16cceeeb655f0a5385b4a8605945a3b6f7fcf473a7132a40f8dc90d204bc5e9e64fd2cc0bdc37dbcabb4ddc5c
c17db32df3 Pub back in deprecated dust_value (Tobin C. Harding)
Pull request description:
When we renamed `dust_value` to `minimal_non_dust` we forgot to keep the original and deprecated it, doing so assists with the upgrade path.
Put back in deprecated `dust_value`, linking to the rename.
Renamed in #2255, found while testing upgrade of downstream software.
ACKs for top commit:
tcharding:
> ACK [c17db32](c17db32df3) I _think_ this matches the behavior of the old version
apoelstra:
ACK c17db32df3 I *think* this matches the behavior of the old version
sanket1729:
ACK c17db32df3
Tree-SHA512: 28e1bd2e1a0fd13c78c70ad2667b72b3bf649c293201b79c86c00f09d0126389ebaeb430b8dd32aeeec3d60cbd8761ae949f5784a5ea7756b1b9ae77ec96ce61
dec05b63e9 Refactor witness_version and is_witness_program (Tobin C. Harding)
dac552b436 Add unit tests for shortest/longest witness program (Tobin C. Harding)
Pull request description:
Refactor `witness_version` and `is_witness_program`.
- Patch 2 adds a couple of preparatory unit tests.
- Patch 2 does the refactor
Fix: #2618
ACKs for top commit:
apoelstra:
ACK dec05b63e9
sanket1729:
ACK dec05b63e9
Tree-SHA512: 3db0a1d8175cbb2fd18f3254854d02db3ad7efa2620b12f08d9727ef6bb5854f0a015917e57023cd2196a36d13276e80536a0e96318c44a1173da4f6793ca370
When we renamed `dust_value` to `minimal_non_dust` we forgot to keep the
original and deprecated it, doing so assists with the upgrade path.
Pub back in deprecated `dust_value`, linking to the rename.
Adds constructors to allow directly creating locktimes from time or
block counts; adds a flooring constructor to Time to match the ceiling
one; adds an explicit constructor to Height since the From<u16> was not
very discoverable.
These two functions are related. We cannot, by definition, get the
witness version from a script that is not a witness program but
currently the code is not linking these two things.
Refactor by doing:
- Move the check of the witness program bip rules to `witness_version`
- Call `witness_version().is_some()` in the predicate
Improve the docs while we are at it to include the bip text in the
rustdoc. Note I didn't bother referencing the segwit bip number, this
bip text is pretty well known.
Add two unit tests that verify we can correctly determine if a
shortest allowed and longest allowed script is a witness program.
Done in preparation for patching the `witness_version` function.
We are currently using the `base58::Error` type to create errors in
`bitcoin`, these are bitcoin errors not `base58` errors.
Note that we add what looks like duplicate
`InvalidBase58PayloadLengthError` types but they are different because
of the expected length. This could have been a field but I elected not
to do so for two reasons:
1. We will need to do so anyways if we crate smash more
2. The `crypto::key` one can have one of two values 33 or 34.
With this applied we can remove the now unused error variants from
`base58::Error`.
b816c0bb01 hash_types: add unit tests for display of all hash types in the library (Andrew Poelstra)
Pull request description:
This can be checked against the 0.29.x branch, and against the commit prior to #1659 (40c246743b^) and you will see that it is consistent EXCEPT:
* In rust-bitcoin 0.29.x we did not have multiple sighash types, only `Sighash`; we now have `LegacySighash`, `SegwitV0Sighash`, and `TapSighash`.
* In #1565 we deliberately changed the display direction of the sighashes, to match BIP 143.
Fixes#2495.
ACKs for top commit:
tcharding:
That's a win. ACK b816c0bb01
Tree-SHA512: 5b44f40165699910ea9ba945657cd1f960cf00a0b4dfa44c513feb3b74cda33ed80d3551042c15b85d6e57c30a54242db202eefd9ec8c8b6e1498b5578e52800
Andrew Poelstra
Fmt Bot
Divyansh Gupta
Tobin C. Harding