Commit Graph

526 Commits

Author SHA1 Message Date
Fmt Bot 747ca578dd 2024-04-07 automated rustfmt nightly 2024-04-07 01:03:23 +00:00
Tobin C. Harding f6467ac98d
Minimize usage of Network in public API
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.
2024-04-03 13:32:39 +11:00
Andrew Poelstra 499f36f972
Merge rust-bitcoin/rust-bitcoin#2337: Add check to max difficulty transition threshold
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
2024-04-02 13:18:29 +00:00
Tobin C. Harding f0f6d3f162
Take Params instead of Network in difficulty function
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`.
2024-04-02 11:41:34 +11:00
Andrew Poelstra 6a2fd96ff6
Merge rust-bitcoin/rust-bitcoin#2581: Implement ArbitraryOrd for relative::LockTime
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
2024-04-02 00:36:12 +00:00
Fmt Bot a565db9fdd 2024-03-31 automated rustfmt nightly 2024-03-31 01:03:18 +00:00
Tobin C. Harding 3520f550f0
Implement ArbitraryOrd for relative::LockTime
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
2024-03-26 10:36:50 +11:00
Andrew Poelstra c211e7be78
Merge rust-bitcoin/rust-bitcoin#2626: Replace TBD with 0.32.0
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
2024-03-24 15:15:09 +00:00
Andrew Poelstra 26248b28ac
Merge rust-bitcoin/rust-bitcoin#2625: Put back in deprecated dust_value
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
2024-03-24 13:56:34 +00:00
Andrew Poelstra 1656e1a09a
Merge rust-bitcoin/rust-bitcoin#2619: Call is_witness_program from witness_version
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
2024-03-22 23:52:42 +00:00
Tobin C. Harding fd040f5e38
Replace TBD with 0.32.0
We are gearing up for the 0.32.0 release; replace all instances of TBD
with the version number of the upcoming release.
2024-03-23 05:36:52 +11:00
Tobin C. Harding c17db32df3
Pub back in deprecated dust_value
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.
2024-03-23 05:32:15 +11:00
Andrew Poelstra 04715e3e60
absolute: make is_* methods uniform with the ones from relative 2024-03-21 22:10:17 +00:00
Andrew Poelstra 878b865f85
relative locktime: introduce is_* methods to check units
Copy these from absolute::LockTime. While we are at it, make the
functions in absolute::LockTime const.
2024-03-21 22:10:17 +00:00
Andrew Poelstra c2f87c7ab3
relative locktime: add is_implied_by method for sequences
This gives a way to determine whether a CSV will pass, given a sequence
number, in a type-safe way where you can't get the two things backward.
2024-03-21 22:10:17 +00:00
Andrew Poelstra 319e102fed
relative locktime: use From/TryFrom to convert between relative locktimes and Sequence 2024-03-21 22:10:17 +00:00
Andrew Poelstra 0ed26915f6
relative locktime: add conversions to/from sequence 2024-03-21 22:10:17 +00:00
Andrew Poelstra 5c8fb5c11b
relative locktime: add consensus encode/decode functions 2024-03-21 22:10:13 +00:00
Andrew Poelstra ac968e02b6
relative locktime: constify a bunch of constructors 2024-03-21 22:07:29 +00:00
Andrew Poelstra f27e675e1e
relative locktime: add "obvious" constructors
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.
2024-03-21 22:07:25 +00:00
Andrew Poelstra f02b1dac5b
relative locktime: copy comments and PartialOrd impl from absolute locktimes 2024-03-21 22:07:25 +00:00
Andrew Poelstra 2ff5085e70
locktimes: run cargo fmt 2024-03-21 22:07:25 +00:00
Tobin C. Harding dec05b63e9
Refactor witness_version and is_witness_program
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.
2024-03-22 07:07:07 +11:00
Tobin C. Harding dac552b436
Add unit tests for shortest/longest witness program
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.
2024-03-22 07:03:30 +11:00
Andrew Poelstra 750b4dfb8b
Merge rust-bitcoin/rust-bitcoin#2569: Move types to `units`
cbee9781e8 Move unit types to units (Tobin C. Harding)
5bd0d7194b Remove unused absolute::Error (Tobin C. Harding)

Pull request description:

  Move the following unit types to the new `units` crate:

  - `locktime::absolute::{Height, Time}`
  - `locktime::relative::{Height, Time}`
  - `FeeRate`
  - `Weight`

  Also move the `parse` module as well as constants as required.

  Do minimal changes to get things building:

  - Feature gate on "alloc" as needed.
  - Remove rustdocs that use `bitcoin` types.
  - Re-export units types so this is a non-breaking change.
  - Fix import paths.

  Patch 1 was originally #2526, putting it in via this PR to try and speed up the process.

  Close: #2282

ACKs for top commit:
  sanket1729:
    ACK cbee9781e8
  apoelstra:
    ACK cbee9781e8 lgtm. this is a good start. I think the LockTime types should follow Height and Time

Tree-SHA512: 6b0d63c7b054008598d7fa81be7d8c112f2778883b5529d79d446617b94b3c196c9ac735f840d1dfb488700894d3161c6976d44ab0e12ac3af4008068eac5f87
2024-03-15 22:43:51 +00:00
Andrew Poelstra 1ceac90bf6
Merge rust-bitcoin/rust-bitcoin#2565: Removes txid prefix in transaction IDs
56132f59d5     Remove the `:#` formatting for `hex_fmt_impl` macro (448 OG)

Pull request description:

  This commit attempts to solve #2505  by ensuring that formatting is not forced using the `:#` in the hex macro code generating in macro rule `hex_fmt_impl` in the hashes/utils.rs file.

  The write! macro forces all formatting to add the prefix `0x` by adding an alternate by (#) default

  ```rust
  impl<$($gen: $gent),*> $crate::_export::_core::fmt::Debug for $ty<$($gen),*> {
              #[inline]
              fn fmt(&self, f: &mut $crate::_export::_core::fmt::Formatter) -> $crate::_export::_core::fmt::Result {
                  write!(f, "{:#}", self) // <-- This is where the formatting is being forced.
              }
          }
  ```

  By removing this formatting, the `:#` must be specified by the user in order for a prefix to be added.

  ```rust
  let outpoint = bitcoin::OutPoint::default();
      println!("{:?}", &outpoint);
      println!("{:#?}", &outpoint);
      println!("{:#}", &outpoint);
      println!("{:x}", &outpoint.txid);
      // `{:#}` must be specified to pretty print with a prefix
      println!("{:#}", &outpoint.txid);
      dbg!(&outpoint);
      dbg!(&outpoint.txid);
  ```

  The PR also adds testcase for this when running `cargo test` .

ACKs for top commit:
  tcharding:
    ACK 56132f59d5
  apoelstra:
    ACK 56132f59d5

Tree-SHA512: 9e4fc9f30ab0b3cf2651d3c09f7f01d8245ac8ea7ae3a82bb4efd19f25c77662bf279020a31fa61b37587cc0c74284696c56045c59f1ba63b2dd42a210d98ebc
2024-03-13 17:28:09 +00:00
Andrew Poelstra d2617f99b2
Merge rust-bitcoin/rust-bitcoin#2530: Improve leaf errors
f8de7954b2 Remove unused pow::TryFromError type (Tobin C. Harding)
43c5eb765c Fix witness_version leaf error type (Tobin C. Harding)
2af764e859 hashes: Fix leaf error type (Tobin C. Harding)

Pull request description:

  In light of recent discussion go over the codebase and look for some places that the leaf errors are wrong. Does not do the whole code base, excludes `p2p` and a couple of other places.

ACKs for top commit:
  apoelstra:
    ACK f8de7954b2
  Kixunil:
    ACK f8de7954b2

Tree-SHA512: 2905878363869ee205cce49c58c060c712c9b7b55965ee60bb856128842968a4be86c93a194ffffdb35e215b2bea8ad33b04ee47e8e17cc784b0641ea48518e5
2024-03-13 15:03:57 +00:00
448 OG 56132f59d5
Remove the `:#` formatting for `hex_fmt_impl` macro
This fixes the issue where pretty debug like `dbg` or `{:#}` introduce the use of
    `0x` prefix to hex encoded transaction ID.

    The transaction id is being forced to pretty print inside the `hex_fmt_impl` macro
    using `{:#}` in the line `write!(f, "{:#}", self)` debug formatter.

    Resolves: #2505
2024-03-12 11:48:05 +03:00
Tobin C. Harding 43c5eb765c
Fix witness_version leaf error type
Leaf error types should typically have private fields, provide accessor
functions, and not use `non_exhaustive`.
2024-03-12 12:14:14 +11:00
Tobin C. Harding cbee9781e8
Move unit types to units
Move the following unit types to the new `units` crate:

- `locktime::absolute::{Height, Time}`
- `locktime::relative::{Height, Time}`
- `FeeRate`
- `Weight`

Also move the `parse` module as well as constants as required.

Do minimal changes to get things building:

- Feature gate on "alloc" as needed.
- Remove rustdocs that use `bitcoin` types.
- Re-export units types so this is a non-breaking change.
- Fix import paths.
2024-03-12 11:59:39 +11:00
Tobin C. Harding 5bd0d7194b
Remove unused absolute::Error
The `absolute::Error` is not used, we originally intended it as possibly
useful for users of the library. We have not made effort in other
modules to provide such errors - lets remove it.
2024-03-12 09:13:09 +11:00
Andrew Poelstra a124ff41c4
Merge rust-bitcoin/rust-bitcoin#2473: Upgrade to `hex v0.2.0`
f337dec2b1 hashes: Remove unnecessary feature guard from test (Tobin C. Harding)
0cea90d505 Test hashes honour Formatter::precision (Tobin C. Harding)
4bfb466bb9 Upgrade hex dependency (Tobin C. Harding)
f0558e8eb9 Use fmt_hex_exact (Tobin C. Harding)
6820f51408 hashes: Add fmt roundtrip tests (Tobin C. Harding)
e302e30e7c Import with super::* in unit test (Tobin C. Harding)

Pull request description:

  Upgrade to use the newly released `hex` code.

  - Patch 1: Does trivial preparatory cleanup
  - Patch 2: Adds some unit tests to check we roundtrip hashes correctly (added because in the test PR I had the `Midstate` iml wrong and it was not being caught).
  - Patch 3: Uses macro in place of `forward_hex` and `backward_hex` - needs concept review, I hacked this without understanding why the functions existed in the first place.
  - Patch 4: Does the upgrade, I've attempted to make minimal changes, so there is room for a bunch of cleanups if/when this merges.
  - Patch 5: Adds a unit test to verify that we can close #2494
  - Patch 6: Removes unnecessary feature gate from unit test.

ACKs for top commit:
  Kixunil:
    ACK f337dec2b1
  apoelstra:
    ACK f337dec2b1

Tree-SHA512: 7913d1b3079cf5ba1b0e70f5c33e091c5ef1258026c8f27bbe8a050100bbc7622b6555d560b15be3b3d90d47ce873f137a73cf2d772108d2915fb30ed129bded
2024-03-11 21:01:24 +00:00
Andrew Poelstra 24b19d7776
Merge rust-bitcoin/rust-bitcoin#2433: Split relative locktime error up
3c8edae25b Split relative locktime error up (Tobin C. Harding)

Pull request description:

  The `relative` module has a single general error type, we are moving away from this style to specific error types.

  Split the `relative::Error` up into three error structs.

  I forget the policy on public inner fields.

ACKs for top commit:
  sanket1729:
    utACK 3c8edae25b
  apoelstra:
    ACK 3c8edae25b

Tree-SHA512: f3079f81a825125f1efe54657fbba64618530b25aecaa3844902900517bf23bec26ff5399cf22f4e63e44316ebb603e8692cbaece2782ecafe09ffed3eab553c
2024-03-11 17:42:54 +00:00
Tobin C. Harding 0cea90d505
Test hashes honour Formatter::precision
Test that the new version of `hex` honours `Formatter::precision` for
new wrapped hash types (ie, types created with `hashes::hash_newtype`).

Fix: #2494
2024-03-10 10:35:02 +11:00
Tobin C. Harding 4bfb466bb9
Upgrade hex dependency
Upgrade to the new `hex v0.2.0` release.
2024-03-10 10:35:01 +11:00
Tobin C. Harding 3c8edae25b
Split relative locktime error up
The `relative` module has a single general error type, we are moving
away from this style to specific error types.

Split the `relative::Error` up into three error structs.

Note the change of parameter `h` to `height`, and using `h` as the
pattern matched variable - this makes sense because it gives the
variable with large scope the longer name.
2024-03-10 09:57:11 +11:00
Sh0g0-1758 08a9962035
Replaced Deprecated Function 2024-03-09 17:18:00 +05:30
Liam Aharon b9f7462958
Implement infallible for errors
Creates a new macro `impl_from_infallible`, and applies it to custom
error types in the codebase.

Closes #1222.
2024-03-08 16:48:34 +11:00
Andrew Poelstra ea6aa99ae4
Merge rust-bitcoin/rust-bitcoin#2492: Remove the FromHexStr trait
b873a3cd44 Do infallible int from hex conversions (Tobin C. Harding)
4d762cb08c Remove the FromHexStr trait (Tobin C. Harding)
026537807f Remove mention of packed (Tobin C. Harding)

Pull request description:

  The `FromHexStr` trait is used to parse integer-like types, however we can achieve the same using inherent methods.

  Move the hex parsing functionality to inherent methods, keeping the same behaviour in regard to the `0x` prefix.

  Patch 1 is trivial preparatory cleanup.

ACKs for top commit:
  apoelstra:
    ACK b873a3cd44
  sanket1729:
    ACK b873a3cd44

Tree-SHA512: a280169b68304fcc1a531cc9ffb6914b70238efc4c2241a766105053911a373a0334b73e5ea3525c331ccb81ce98c43fea96dae77668804e608376a48d5ed8ac
2024-03-07 14:36:53 +00:00
geekvest a6adfd845c fix some comments
Signed-off-by: geekvest <cuimoman@sohu.com>
2024-03-03 13:41:23 +08:00
Tobin C. Harding b873a3cd44
Do infallible int from hex conversions
We have three integer wrapping types that can be created from hex
strings where the conversion from an integer is infallible:

- `absolute::LockTime`
- `Sequence`
- `CompactTarget`

We would like to improve our handling of the two prefix characters (eg
0x) by making it explicit.

- Modify the inherent `from_hex` method on each type to error if the
input string does not contain a prefix.

- Add an additional inherent method on each type `from_unprefixed_hex`
that errors if the input string does contain a prefix.

This patch does not touch the wrapper types that cannot be infallibly
constructed from an integer (i.e. absolute `Height` and `Time`).
2024-02-27 10:40:52 +11:00
Tobin C. Harding 4d762cb08c
Remove the FromHexStr trait
The `FromHexStr` trait is used to parse integer-like types, however we
can achieve the same using inherent methods.

Move the hex parsing functionality to inherent methods, keeping the same
behaviour in regard to the `0x` prefix.
2024-02-27 10:09:20 +11:00
Tobin C. Harding 026537807f
Remove mention of packed
We removed the `PackedLockTime`, remove all mentions of the word packed.
2024-02-27 10:09:20 +11:00
Tobin C. Harding ac88bc03fd
Make constructors const
Audit the codebase for any function that starts with `/// Creates` and
see if we can make it const. Inline them at the same time.
2024-02-24 06:04:41 +11:00
Tobin C. Harding 9187bf3a65
Fix new nightly warnings/errors
The latest nightly toolchain introduced a whole bunch of new warnings
and errors, mostly to do with import statements - fix them all.
2024-02-21 14:13:49 +11:00
yancy 1d13020129 test: Add conditional check for debug_assertions
Adding debug_assertions where the test case panics allows the test suite
to run in release mode successfully.
2024-02-10 12:37:50 +01:00
Tobin C. Harding 10cf51c4c5
Inline private ScriptBuf::p2wpkh function
This function is a bit unclear and is only called once, just inline it.

Refactor only, no logic changes.
2024-02-07 10:09:02 +11:00
Andrew Poelstra 530899ae38
Merge rust-bitcoin/rust-bitcoin#2445: Add API functions `p2wpkh_script_code`
3c62f74684 Add public functions p2wpkh_script_code (Tobin C. Harding)
a246dc98a4 Run sighash example in CI (Tobin C. Harding)

Pull request description:

  This was done to fix #1920, it may be of questionable value though.

  - Patch 1 is definitely useful, its a CI fix.
  - Patch 2 adds two new API functions.

  Fix: #1920

ACKs for top commit:
  Kixunil:
    ACK 3c62f74684
  apoelstra:
    ACK 3c62f74684

Tree-SHA512: 58743612c48e392f9ac0a94477588aee959c5fe9191dd04405bbb71aed7b0730b5927ad98f9da34dc93caaaac939617348c3f71318cc7e65c2c154b0f3897b89
2024-02-06 14:06:11 +00:00
Andrew Poelstra 0f669404c4
Merge rust-bitcoin/rust-bitcoin#2443: Print hex in Debug for Sequence
c084afa8b2 Print hex in Debug for Sequence (Tobin C. Harding)

Pull request description:

  Printing the `Sequence` as a decimal is not super useful when debugging, print it in hex instead.

  Using code:

          let seq = Sequence::from_consensus(0xFFFFFFFF);
          println!("sequence: {:?}", seq);

  Before applying this patch we get:

          sequence: Sequence(4294967295)

  And after applying we get:

          sequence: Sequence(0xffffffff)

ACKs for top commit:
  Kixunil:
    ACK c084afa8b2
  apoelstra:
    ACK c084afa8b2

Tree-SHA512: d60cd8896ca56a30fc8bd030cf3dd1bc1fd3a1609e99bfc2f26b9bd665b11c34c9df93b3f3ad731506d916513ca4a192dde476e16d99f2d4c4b2697f70a7bc98
2024-02-06 13:45:35 +00:00
Tobin C. Harding 3c62f74684
Add public functions p2wpkh_script_code
Add two public API functions on the two public keys, both called
`p2wpkh_script_code` to do exactly as the name suggests.

Of note, I was not able to find anywhere to use these in example code,
this is because of we always use the new `p2wpkh_signature_hash`
function. The new functions may be useful for a user calling
`segwit_v0_encode_signing_data_to`. The may help document the library as
well.
2024-02-06 14:35:54 +11:00
Tobin C. Harding c084afa8b2
Print hex in Debug for Sequence
Printing the `Sequence` as a decimal is not super useful when debugging,
print it in hex instead.

Using code:

        let seq = Sequence::from_consensus(0xFFFFFFFF);
        println!("sequence: {:?}", seq);

Before applying this patch we get:

        sequence: Sequence(4294967295)

And after applying we get:

        sequence: Sequence(0xffffffff)
2024-02-06 12:25:37 +11:00
Tobin C. Harding 8c17ad7fd7
Remove non_exhaustive from struct errors with pub inner
Using `non_exhaustive` as well as a public inner field is incorrect, it
prohibits users from creating or matching on the error and does not
achieve forward comparability.

This was never right, we shouldn't have done it.
2024-02-05 16:26:31 +11:00
Tobin C. Harding c69caafefc
Remove attribute comments
Add an issue and remove the TODO from the code as well as the attribute
comments, leave a single comment as an explanation of why the unusual
code block.

ref: https://github.com/rust-bitcoin/rust-bitcoin/issues/2427
2024-02-02 06:22:02 +11:00
Martin Habovstiak 93dba898c2 Improve lock time errors
The errors returned from various lock time functions had several issues.
Among the obvious - `Error` being returned from all operations even when
some of its variants were unreachable, there were subtle issues around
error messages:

* `ParseIntError` didn't contain information whether the parsed object
   is `Height` or `Time`.
* Logically overflow and out-of-bounds should be the same thing but
  produced different error messages.
* Mentioning integers is too technical for a user, talking about upper
  and lower bound is easier to understand.
* When minus sign is present `std` reports it as invalid digit which is
  less helpful than saying negative numbers are not allowed.

It is also possible that `ParseIntError` will need to be removed from
public API during crate smashing or stabilization, so avoiding it may be
better.

This commit significantly refactors the errors. It adds separate types
for parsing `Height` and `Time`. Notice that we don't compose them from
`ParseIntError` and `ConversionError` - that's not helpful because they
carry information that wouldn't be used when displaying which is
wasteful. Keeping errors small can be important.

It's also worth noting that exposing the inner representation could
cause confusion since the same thing: out of bounds can be represented
as an overflow or as a conversion error. So for now we conservatively
hide the details and even pretend there's no `source` in case of
overflow. This can be expanded in the future if needed.

The returned errors are now minimal. `LockTime` parsing errors are
currentlly unchanged.
2024-01-31 15:13:56 +01:00
Martin Habovstiak dda83707a2 Use `unsigned_abs` instead of manual code
The code originally used `if` and incorrectly casted the value into
`usize` rather than `u64`. This change replaces the whole thing with
`unsigned_abs`.

Closes #1247
2024-01-27 20:49:15 +01:00
Andrew Poelstra 6bd8375959
Merge rust-bitcoin/rust-bitcoin#2402: Remove TODOs
5eb2de1660 Remove TODO about rand trait (Tobin C. Harding)
66cc007c2b p2p: Remove TODO comments (Tobin C. Harding)
0b5fb45ea0 consensus: Remove HEX_BUF_SIZE todo (Tobin C. Harding)
579668892a consensus: Remove TODO (Tobin C. Harding)
53beb9db30 Remove ancient todos in test code (Tobin C. Harding)
abe2241828 units: Remove "alloc" TODO (Tobin C. Harding)
5386ef0fd2 psbt: Delete TODO comments (Tobin C. Harding)
14c8a2232b examples: Remove TODO (Tobin C. Harding)

Pull request description:

  Done while working on #2368.  There are 5 left. Do we want to leave the MSRV ones in there?

  ```bash
  bitcoin/src/blockdata/weight.rs:66:                 // TODO replace with panic!() when MSRV = 1.57+
  bitcoin/src/consensus/serde.rs:101:    // TODO: statically prove impossible cases
  bitcoin/src/pow.rs:445:            // TODO: Use `carrying_mul` when stabilized: https://github.com/rust-lang/rust/issues/85532
  units/src/amount.rs:595:        // TODO replace whith unwrap() when available in const context.
  units/src/amount.rs:599:                // TODO replace with panic!() when MSRV = 1.57+
  ```

ACKs for top commit:
  Kixunil:
    ACK 5eb2de1660
  apoelstra:
    ACK 5eb2de1660

Tree-SHA512: 285b1711a6e6fba126e2c4159b25454c7f894122b76fde1d3d29e57b2ec0a6e90230e46ac79d70aa133da177c75d267fc5a13489b69881862649de771027ec8e
2024-01-25 15:06:28 +00:00
Andrew Poelstra 2de220ec6a
Merge rust-bitcoin/rust-bitcoin#2097: Add `Witness::p2tr_key_spend` function
6715e93e89 Add Witness::p2tr_key_spend function (Tobin C. Harding)

Pull request description:

  Add a function for creating the witness when doing a key path spend for a P2TR output.

  This mirrors what we did for P2WPKH when adding `Witness::p2wpkh`.

  Includes update to the taproot signing example to use the new constructor.

ACKs for top commit:
  Kixunil:
    ACK 6715e93e89
  apoelstra:
    ACK 6715e93e89

Tree-SHA512: aab51329e8fda471442bb9cebd6327636548dd157bb9842fe66993fcdd211bb04b2b829aa9d5962dd619f5c0b73d19644a44529c1a5958df1a6bc892147b44f5
2024-01-25 13:34:06 +00:00
Tobin C. Harding 53beb9db30
Remove ancient todos in test code
These todos has been here since 2015 and 2016- I don't think they are
getting done anytime soon, just remove them.
2024-01-25 16:59:55 +11:00
Andrew Poelstra 48b42c10fb
Merge rust-bitcoin/rust-bitcoin#2395: Do minor error fixes
fb81bff61f Add a from impl for ParseIntError (Tobin C. Harding)
2130150df6 absolute: Use Self in error type (Tobin C. Harding)

Pull request description:

  While reviewing #2335 I noticed a few places that error code needed some love.

ACKs for top commit:
  Kixunil:
    ACK fb81bff61f
  Harshit933:
    ACK [`fb81bff`](fb81bff61f)
  apoelstra:
    ACK fb81bff61f

Tree-SHA512: 17f4e448862be47534d4c1c2962d5db8f90a471e53226022d7c2e153fc501705cd65b070eb17f3239fb8ad242223340f78fe828ea7df3d43707d3e42fcbef557
2024-01-24 22:17:46 +00:00
Andrew Poelstra cf3a7bb758
Merge rust-bitcoin/rust-bitcoin#2366: Rename `txid` to `compute_txid`
7af3a58e91 Rename ntxid to compute_ntxid (yancy)
9bbf79b08c Rename wtxid to compute_wtxid (yancy)
57a7613973 Rename txid to compute_txid (yancy)

Pull request description:

  Rename txid to compute_txid and mark txid as deprecated.

  Closes #2363

ACKs for top commit:
  Kixunil:
    ACK 7af3a58e91
  apoelstra:
    ACK 7af3a58e91
  tcharding:
    ACK 7af3a58e91

Tree-SHA512: 0d9200588cd83c42b78f0ba5e6a6aa049e8360aec8b9881cf9bfbaaad0d256e6879b494fb3c4e2b06d371b9e5dde6addcc94c67a080d16a6eed5c9bc36bc845a
2024-01-24 14:07:03 +00:00
Tobin C. Harding 6715e93e89
Add Witness::p2tr_key_spend function
Add a function for creating the witness when doing a key path spend for
a P2TR output.

This mirrors what we did for P2WPKH when adding `Witness::p2wpkh`.

Includes update to the taproot signing example to use the new constructor.
2024-01-24 15:21:31 +11:00
Tobin C. Harding fb81bff61f
Add a from impl for ParseIntError
As is customary add a `From` impl for the `ParseIntError` and use `?`.
While this does not make much difference it saves devs wondering why
there is a `From` impl for one of the variants and not the other.
2024-01-24 13:31:57 +11:00
Tobin C. Harding 2130150df6
absolute: Use Self in error type
As is becoming conventional here, use `Self` in the `From` impl for
error type.
2024-01-24 13:29:20 +11:00
yancy 7af3a58e91 Rename ntxid to compute_ntxid
Computing the ntxid is computationally expensive, so rename the method
accordingly.
2024-01-23 19:53:39 +01:00
yancy 9bbf79b08c Rename wtxid to compute_wtxid
Computing the wtxid is computationally expensive, so rename the method
accordingly.
2024-01-23 15:57:50 +01:00
yancy 57a7613973 Rename txid to compute_txid
Computing the txid is computationally expensive, so rename the method
accordingly.
2024-01-23 15:57:50 +01:00
Tobin C. Harding dae16f052c
Use any method on iterator
We have `Iterator::any` which better shows whats going on in this loop.

Refactor only, no logic changes.
2024-01-23 10:48:33 +11:00
Tobin C. Harding 671dc0e9e0
Use better predicate name
This function is a predicate, it should have identifier `uses` not
`use`.
2024-01-23 10:46:23 +11:00
Andrew Poelstra ff51619c79
Merge rust-bitcoin/rust-bitcoin#2364: Test: add invalid segwit transaction test
fe8d559d69 test: add invalid segwit transaction test (startup-dreamer)

Pull request description:

  Tries to close #2183

  Added the test for invalid segwit transaction (witness flag is set but no witness is present) using [This suggested hex](https://github.com/rust-bitcoin/rust-bitcoin/issues/2183#issuecomment-1901207149) by Kixunil

ACKs for top commit:
  Kixunil:
    ACK fe8d559d69
  apoelstra:
    ACK fe8d559d69

Tree-SHA512: 723027e0ad9944a4763fba1e12398d7bcacdef691a40168f0be7cecdb170936f7e0c3690c4de911d086b8c5d42f5a25784f53fe096404f5cf69d6fc75c645d6e
2024-01-21 14:45:34 +00:00
startup-dreamer fe8d559d69
test: add invalid segwit transaction test 2024-01-19 23:12:08 +05:30
Andrew Poelstra 01c8f2021e
Merge rust-bitcoin/rust-bitcoin#2358: Remove quadratic algorithm
a338a61cc3 Remove quadratic algorithm (Tobin C. Harding)

Pull request description:

  Currently we loop over transaction inputs within a loop over transaction inputs - ouch.

  Cache the `use_segwit_serialization` outside the iteration loop.

  Fix: #2357

ACKs for top commit:
  Kixunil:
    ACK a338a61cc3
  apoelstra:
    ACK a338a61cc3

Tree-SHA512: 91d0b46b235db57d9c28fc8da5d43a52c76a29916797a4ec44273b91eb120a928050a79cdbd704b922635dd2130db7b6e7863fd10e878eee52882c661af54c11
2024-01-19 15:27:06 +00:00
Andrew Poelstra 111094ca9e
Merge rust-bitcoin/rust-bitcoin#2329: Improve error handling in the `sighash` module
e356ff6611 Remove the now unused sighash::Error type (Tobin C. Harding)
c17324c574 Introduce segwit sighash error types (Tobin C. Harding)
f0b567313b Introduce sighash::LegacyError (Tobin C. Harding)
a1b21e2f1d Introduce sighash::TaprootError (Tobin C. Harding)
b0f20903a5 Introduce AnnexError (Tobin C. Harding)
a1a2056829 Add tx_in/tx_out accessor methods on Transaction (Tobin C. Harding)
f08aa16e91 Use Self:: in error return type (Tobin C. Harding)

Pull request description:

  Improve the error handling in the `sighash` module by adding small specific error types.

  Close: #2150

ACKs for top commit:
  Kixunil:
    ACK e356ff6611
  apoelstra:
    ACK e356ff6611

Tree-SHA512: e2e98a4caccae4e4acdc0e577e369fc90ee39a2206a8a1451739695fbe33ec2c3a52482b70cec8f9ee6bdb3ad7a2f4f639e8c87031878cd5d816fae24d913c42
2024-01-19 15:11:29 +00:00
Andrew Poelstra 783ba73799
Merge rust-bitcoin/rust-bitcoin#2356: Use full path in all macro usage of Result
61bf462806 Use full path in all macro usage of Result (josibake)

Pull request description:

  Follow-up to https://github.com/rust-bitcoin/rust-bitcoin/pull/2355

  Couldn't think of a clever way to do this , so just grepped for all instances of `macro_rules` and added the full path for the imports. Wasn't sure if it was necessary for `fmt::Result`, but went ahead and added the full path for consistency.

  Tested locally and confirmed this fixes the issue I was seeing.

ACKs for top commit:
  apoelstra:
    ACK 61bf462806
  Kixunil:
    ACK 61bf462806
  tcharding:
    ACK 61bf462806

Tree-SHA512: 8af105b3e6a36723804b290f8254f52e65cd42a61c323f1190e3bcbcb9e4427ff9b026a4530bafcd14aab644ccd9401fed351494457194c3a68a55f11b2a3d04
2024-01-19 13:48:33 +00:00
Tobin C. Harding a1a2056829
Add tx_in/tx_out accessor methods on Transaction
In a few places in the codebase we want to grab an reference to an input
by index. To reduce code duplication add two methods on `Transaction`,
each to get a reference to an input or output respectively.

These are public methods, do not use them yet internally.
2024-01-19 12:21:25 +11:00
Tobin C. Harding a338a61cc3
Remove quadratic algorithm
Currently we loop over transaction inputs within a loop over transaction
inputs - ouch.

Cache the `use_segwit_serialization` outside the iteration loop.

Fix: #2357
2024-01-19 11:58:19 +11:00
Tobin C. Harding bd41c836ab
Fix cut'n'pasta error in map variable
We are mapping outputs not inputs.
2024-01-19 07:36:11 +11:00
josibake 61bf462806
Use full path in all macro usage of Result
If a user has defined their own alias for Result and tries to use a macro,
relative paths cause an issue. Use full paths to fix this.
2024-01-18 19:11:01 +01:00
Andrew Poelstra 2073a40c50
Merge rust-bitcoin/rust-bitcoin#2240: Require `BufRead` instead of `Read`
263a8b3603 Require BufRead instead of Read (Tobin C. Harding)
32d68fd1fa io: Add BufRead trait (Tobin C. Harding)

Pull request description:

  Require `BufRead` instead of `Read` for consensus decode trait.

ACKs for top commit:
  Kixunil:
    ACK 263a8b3603
  apoelstra:
    ACK 263a8b3603

Tree-SHA512: 58ad04c7267f9091738463331473bd22b61e6b06a13aec38b3602a369cd8e571d7d1388fd81dd7a0a05f2e8d5a9c35270cd8a918a4fafe636506591ed06a4cb2
2024-01-16 15:16:54 +00:00
Andrew Poelstra a3d698ac7c
Merge rust-bitcoin/rust-bitcoin#2230: Add effective value calculation
d69d62822d Add effective_value method (yancy)

Pull request description:

  Draft PR for adding effective value calculation to TxOut.  Adding this method was discussed here: https://github.com/rust-bitcoin/rust-bitcoin/pull/2217

ACKs for top commit:
  Kixunil:
    ACK d69d62822d
  apoelstra:
    ACK d69d62822d

Tree-SHA512: e74788fea2f97a10ccf2015406178bdac99a54207fe51f0c830937d2106d564beeebe6f70b49aded572aa17ccb42f47e25905a8f791bd9b442eff223ea59baae
2024-01-16 14:24:39 +00:00
Tobin C. Harding 263a8b3603
Require BufRead instead of Read
Our decoding code reads bytes in very small chunks. Which is not
efficient when dealing with the OS where the cost of a context switch is
significant. People could already buffer the data but it's easy to
forget it by accident.

This change requires the new `io::BufRead` trait instead of `io::Read`
in all bounds.

Code such as `Transaction::consensus_decode(&mut File::open(foo))` will
break after this is applied, uncovering the inefficiency.

This was originally Kix's work, done before we had the `io` crate.
Changes to `bitcoin` were originally his, any new mistakes are my own.
Changes to `io` are mine.

Co-developed-by: Martin Habovstiak <martin.habovstiak@gmail.com>
2024-01-16 14:36:00 +11:00
yancy d69d62822d Add effective_value method
The effective_value method is useful for coin selection algorithms.  By
providing this effective value method, the effective value of each
output can be known during the coin selection process.
2024-01-16 00:29:26 +01:00
Andrew Poelstra 6c94546360
Merge rust-bitcoin/rust-bitcoin#2248: Implement ArbitaryOrd for absolute::LockTime
518f0970c9 Implement ArbitaryOrd for absolute::LockTime (Tobin C. Harding)

Pull request description:

  At times we would like to provide types that do not implement `PartialOrd` and `Ord` because it does not make sense. I.e we do not want users writing `a < b`. This could range from kind-of-iffy to down-right-buggy (like comparing absolute locktimes).

  However this decision effects downstream users who may not care about what the ordering means they just need to use it for some other reason e.g., to use as part of a key for a `BTreeMap` (as we do in `miniscript` requiring the `AbsLockTime` type).

  A solution to this problem is to provide a wrapper data type that adds `PartialOrd` and `Ord` implementations. I wrote the `ordered` crate is for this very purpose.

ACKs for top commit:
  apoelstra:
    ACK 518f0970c9
  Kixunil:
    ACK 518f0970c9

Tree-SHA512: 05c753e650b6e2f181caf7dc363c4f8ec89237b42883bd695a64da0661436c9a7e715347f8fcf4fb19ce069cbf75a93032052e946f05fd8029f61860cf9c6225
2024-01-15 16:03:10 +00:00
Tobin C. Harding 271b45299f
Improve Signature field names
Applies to both `ecdsa::Signature` and `taproot::Signature`.

Re-name the `Signature` fields with more descriptive names. The
names used were decided upon in the issue discussion.

Impove rustdocs while we are at it.

Note, the change to `sign-tx-segwit-v0` is refactor only, the diff does
not show it but we have a local variable already called `sighash_type`
that is equal to `EcdsaSighashType::All`.

Includes a function argument rename as well, just to be uniform.

Fix: #2139
2024-01-15 10:26:40 +11:00
Fmt Bot e768c92ce3 2024-01-14 automated rustfmt nightly 2024-01-14 01:00:10 +00:00
GoodDaisy ed47c35b4d Fix typos in rustdocs 2024-01-11 22:45:45 +08:00
Tobin C. Harding 518f0970c9
Implement ArbitaryOrd for absolute::LockTime
At times we would like to provide types that do not implement
`PartialOrd` and `Ord` because it does not make sense. I.e., we do not
want users writing `a < b`. This could range from kind-of-iffy to
down-right-buggy (like comparing absolute locktimes).

However this decision effects downstream users who may not care about
what the ordering means they just need to use it for some other reason
e.g., to use as part of a key for a `BTreeMap` (as we do in `miniscript`
requiring the `AbsLockTime` type).

A solution to this problem is to provide a wrapper data type that adds
`PartialOrd` and `Ord` implementations. I wrote the `ordered` crate is
for this very purpose.

Feature gate a new dependency on `ordered` and implement `ArbitraryOrd`
for `absolute::LockTime`.
2024-01-09 13:15:29 +11:00
Andrew Poelstra de9f20a620
Merge rust-bitcoin/rust-bitcoin#2321: Derive `Copy` for `WitnessProgram`
b02c7d1d33 Derive Copy for WitnessProgram (Tobin C. Harding)

Pull request description:

  Recently we started using our custom `ArrayVec` for the `program` field of `WitnessProgram`, this means we can now derive `Copy`.

  Fix: #2313

ACKs for top commit:
  apoelstra:
    ACK b02c7d1d33
  sanket1729:
    ACK b02c7d1d33

Tree-SHA512: 5741081d578f7b056c156d046dc3b0817b4b13cf69dcc1dfb8c7f4dbe8a4f9ed6c8802aaaf2b0084dbf3984d3fde807a02dbaa8c3bd29c220b3b32d3cb7c9f38
2024-01-08 20:44:06 +00:00
Andrew Poelstra 8aab550e97
Merge rust-bitcoin/rust-bitcoin#2322: Remove Push enum
a8d50a5541 Remove Push enum (Tobin C. Harding)

Pull request description:

  The `Push` enum is only ever used to get access to one of its variants. Since it is a private type we can remove it entirely and just return `PushBytes` from the `last_pushdata` function.

  Needs careful review but I believe the function name is still correctly descriptive.

  This was discovered by of a new nightly clippy warning.

ACKs for top commit:
  apoelstra:
    ACK a8d50a5541 Looks good to me. The latest compiler complains about the currently-unused variants.
  sanket1729:
    ACK a8d50a5541.

Tree-SHA512: 7f96057b0f6f5673252578253ad4f1789793dbf6e917d3974274dedf942da27e6247946262a0669eb500d47987788fcca0e020ed16c0d672188e95ee31163242
2024-01-08 20:29:02 +00:00
Andrew Poelstra 6aaaae6ffc
Merge rust-bitcoin/rust-bitcoin#2294: Deprecate `Script::is_provably_unspendable`
089ce8f0fb Deprecate `Script::is_provably_unspendable` (Martin Habovstiak)

Pull request description:

  This method is not really that useful because it checked an arbitrary condition. There already exists `OP_RETURN` semantics and the method didn't cover all possible ways the script may be invalid.

  This deprecates the method and documents why.

  Closes #2191

ACKs for top commit:
  apoelstra:
    ACK 089ce8f0fb
  tcharding:
    ACK 089ce8f0fb
  sanket1729:
    ACK 089ce8f0fb

Tree-SHA512: 044f1c06fb8cbea4f84817be41bf10315f690b2a42748a07c1dd1eb0ba10932456780956fc628fec4bf57fe0722129537874a77be482d6660f9e02de5fc5a8a0
2024-01-08 15:12:50 +00:00
Tobin C. Harding a8d50a5541
Remove Push enum
The `Push` enum is only ever used to get access to one of its variants.
Since it is a private type we can remove it entirely and just return
`PushBytes` from the `last_pushdata` function.

Needs careful review but I believe the function name is still correctly
descriptive.
2024-01-08 14:04:55 +11:00
Tobin C. Harding b02c7d1d33
Derive Copy for WitnessProgram
Recently we started using our custom `ArrayVec` for the `program` field
of `WitnessProgram`, this means we can now derive `Copy`.

Fix: #2313
2024-01-08 13:51:12 +11:00
Fmt Bot 774b405ba9 2024-01-07 automated rustfmt nightly 2024-01-07 00:59:57 +00:00
shuoer86 3568b9b546
Fix typos 2024-01-05 23:10:31 +08:00
conduition 01df1417c7
use arrayvec to represent witness programs 2024-01-03 17:10:57 +00:00
Andrew Poelstra 471da86e5a
Merge rust-bitcoin/rust-bitcoin#2309: Add the implementation of `Display` for `transaction::Version`
429a3ecec4 Add the implementation of `Display` for `transaction::Version` (harshit933)

Pull request description:

  Adds the implementation of `Display` trait for `transaction::Version`

  fixes #2308

  This is unrelated to the issue but can anyone suggest some good issues that needs to be fixed. I am also taking a look but I am confused as to which I would be able to solve. I am here to learn more.
  Thank you.

ACKs for top commit:
  apoelstra:
    ACK 429a3ecec4 Merry Christmas
  tcharding:
    ACK 429a3ecec4

Tree-SHA512: 9e59a8fe494b01caa8f211441744709f26df03891be171242bea4f7ccd7c3cc58b548cad241cab5270ad66fc9bb33ea7d6f98cc60d496c47647fb3396db9410f
2024-01-02 16:37:56 +00:00
yancy 278229def5 Add allow for out of bounds indexing
Out of bounds indexing is a workaround for const panic until MSRV +1.57
2024-01-01 10:35:52 +01:00
harshit933 429a3ecec4 Add the implementation of `Display` for `transaction::Version` 2023-12-22 21:08:26 +05:30
Martin Habovstiak 5fd731f095
Don't match on complex expression
Passing a complex expression to `match` is hard to read. Assign it to a
variable first.
2023-12-19 19:28:44 +00:00
Fmt Bot 5af7727250 2023-12-17 automated rustfmt nightly 2023-12-17 00:59:05 +00:00
Martin Habovstiak 089ce8f0fb Deprecate `Script::is_provably_unspendable`
This method is not really that useful because it checked an arbitrary
condition. There already exists `OP_RETURN` semantics and the method
didn't cover all possible ways the script may be invalid.

This deprecates the method and documents why.
2023-12-15 23:55:21 +01:00