Commit Graph

245 Commits

Author SHA1 Message Date
Andrew Poelstra 5f2beb8ae2
Merge rust-bitcoin/rust-bitcoin#1527: Rename `push_scriptint` and make it private
94b678e73f Rename `push_scriptint` and make it private (Martin Habovstiak)

Pull request description:

  `push_scriptint` is a significant footgun with an unclear name. This renames it and unpublishes to avoid mistakes by downstream crates.

  Closes #1517

ACKs for top commit:
  apoelstra:
    ACK 94b678e73f

Tree-SHA512: 9e1772c6fb326d8b0c78d702ad9926a79a91589feb8650aed7c5e75bfbdbf0164357b4d5b190877c40b8469e0e3be3d3453fe407741b5dae0c5758176f756417
2023-01-06 14:57:00 +00:00
Andrew Poelstra d71c428949
Merge rust-bitcoin/rust-bitcoin#1528: [WIP] 1514: fix formatting in opcodes
77799bf5cb 1514: fix formatting in opcodes, unify definition and display (bnabi)

Pull request description:

  See  #1514.

ACKs for top commit:
  Kixunil:
    ACK 77799bf5cb
  apoelstra:
    ACK 77799bf5cb

Tree-SHA512: beab4e5ff0f2c995cfb120e5aa04fdeda740ce5136e60da390cf0156c372e3807263a8cdf3eea2762a0fb68b222fed56efdc5ad686b9f481fabe3fccf227fed3
2023-01-06 13:53:21 +00:00
bnabi 77799bf5cb 1514: fix formatting in opcodes, unify definition and display 2023-01-05 14:28:20 +05:30
Martin Habovstiak 94b678e73f Rename `push_scriptint` and make it private
`push_scriptint` is a significant footgun with an unclear name. This
renames it and unpublishes to avoid mistakes by downstream crates.

Closes #1517
2023-01-04 22:58:27 +01:00
Casey Rodarmor 16c49df688 Accept amounts with denominations with and without spaces 2023-01-04 11:07:03 -08:00
Andrew Poelstra deaf21dd84
Merge rust-bitcoin/rust-bitcoin#988: Replace consensus `Encodable`/`Decodable` Psbt Serialization
c4363e5ba1 Deserialize Psbt fields, don't consensus_encode (DanGould)
c1dd6ad8a2 Serialize Psbt fields, don't consensus_encode them (DanGould)
1b7b08aa5d De/serialize Psbt without consensus traits (DanGould)

Pull request description:

  fix https://github.com/rust-bitcoin/rust-bitcoin/issues/934

  Instead of using consensus {de,en}code, serialize high-level structures (PartiallySignedTransaciton, Output, Input) borrow the signature from `Serialize`, `Deserialize` traits and implement them on Psbt:

  ```rs
  impl Psbt {
      /// Serialize a value as raw data.
      fn serialize(&self) -> Vec<u8>;

      /// Deserialize a value from raw data.
      fn deserialize(bytes: &[u8]) -> Result<Self, encode::Error>;
  }
  ```

ACKs for top commit:
  apoelstra:
    ACK c4363e5ba1
  tcharding:
    ACK c4363e5ba1
  sanket1729:
    ACK c4363e5ba1. One small comment, but can be addressed in follow up if there is nothing else from other reviewers.
  Kixunil:
    ACK c4363e5ba1

Tree-SHA512: d8dd5ef1189b36fde08969b1ec36006a6fc55ae990f62ea744e6669e03a7f9e90e1d5907be7eac48ee1af23bc20a62aa7e60ff1ff78080d0e923bb9ccedcd432
2023-01-04 18:32:59 +00:00
sanket1729 98203bc8b3
Merge rust-bitcoin/rust-bitcoin#1438: Replace `Vec::from_hex` with `hex!`
089a1e452d Replace `Vec::from_hex` with `hex!` (Martin Habovstiak)

Pull request description:

  This makes the code less noisy and is a preparation for changing it to `const`-based literal. Because of the preparation, places that used variables to store the hex string were changed to constants.

  There are still some instances of `Vec::from_hex` left - where they won't be changeable to `const` and where `hex!` is unavailable (integration tests). These may be dealt with later.

  See also #1189

  Note that while the change appears big it's nearly entirely mechanical, so should be pretty easy to review. (But I don't feel it's `trivial`.)

ACKs for top commit:
  apoelstra:
    ACK 089a1e452d
  tcharding:
    ACK 089a1e452d

Tree-SHA512: c55357b8cffc86f8e107c0f8390490ede75948260018f63dc926455700cbcaf422f6ae72444f93943065e6f4ffa4335bee9160a64184ea4f8e91721f30a46ace
2023-01-01 02:24:01 -08:00
Martin Habovstiak 089a1e452d Replace `Vec::from_hex` with `hex!`
This makes the code less noisy and is a preparation for changing it to
`const`-based literal. Because of the preparation, places that used
variables to store the hex string were changed to constants.

There are still some instances of `Vec::from_hex` left - where they
won't be changeable to `const` and where `hex!` is unavailable
(integration tests). These may be dealt with later.

See also #1189
2022-12-31 21:10:19 +01:00
Martin Habovstiak 8d58ee2ca3 Change `#[cfg(docsrs)]` to `#[cfg(doc)]` on `use`
The additional `use` items were added to improve the style of
documentation. Because they were only used for doc they were `cfg`ed.
But because this is independent from being built by `docs.rs` the `cfg`
should've been `doc` not `docsrs`.

IOW `docsrs` means roughly `all(doc, nightly)` and the added items are
unrelated to `nightly`.
2022-12-31 20:58:17 +01:00
Andrew Poelstra 3b11ff6607
Merge rust-bitcoin/rust-bitcoin#1387: Add rand-std feature
941083ec4e Remove rand-std dev-dependency from secp256k1 (Tobin C. Harding)
f71335f971 Add rand-std feature (Tobin C. Harding)

Pull request description:

  This PR uncovered incorrect feature gating in `secp256k1`, fixed in https://github.com/rust-bitcoin/rust-secp256k1/pull/519

  Currently we enable "secp256k1/rand-std" in the "rand" feature, this is incorrect because it means "rand" implies "std" which it does not.

  Add a "rand-std" feature that turns on "seck256k1/rand-std" and make the "rand" feature turn on "seck256k1/rand".

  Fix: #1384

ACKs for top commit:
  sanket1729:
    ACK 941083ec4e
  apoelstra:
    ACK 941083ec4e

Tree-SHA512: e1d12196766c2b3082b488fe7d0c03a865ebbfc70ffa6f128c57074df14da71e56c31ea92982991d376ac62fa86c6639049ce17aac93613b523ddcc99677c269
2022-12-30 23:53:54 +00:00
Andrew Poelstra 3a867ee8ce
Merge rust-bitcoin/rust-bitcoin#1503: Fix wrong newtype APIs
64f7d2549e Fix wrong newtype APIs (Martin Habovstiak)

Pull request description:

  This fixes several API bugs:

  * Use `TryFrom` instead of `From` for fallible conversions
  * Move byte conversion methods from `impl_array_newtype` to `impl_bytes_newtype`
  * Add missing trait impls like `AsRef`, `Borrow`, their mutable versions and infallible conversions from arrays

  Closes #1336

  Notably this change is much less bad than even I expected and some places actually get cleaner than before.

ACKs for top commit:
  tcharding:
    ACK 64f7d2549e
  sanket1729:
    code review ACK 64f7d2549e
  apoelstra:
    ACK 64f7d2549e

Tree-SHA512: 0c5ab19aa1a01ccf34ad1c2f3f66bd003a3142a840cb7cf57e1449e721f6eed1523555c62f0e3391012708767ff86315ff7edce9afd85017a67c61cfe57dddbc
2022-12-30 23:10:39 +00:00
Andrew Poelstra 3e4a299615
Merge rust-bitcoin/rust-bitcoin#1513: Add mutation testing to the `locktime` module
ca471557a5 locktime: Add mutation testing (Tobin C. Harding)
26c0da41b4 locktime: Add inline to public functions (Tobin C. Harding)

Pull request description:

  Add mutation testing to the `locktime` module and add unit tests to cover all mutants and ensure they are killed.

ACKs for top commit:
  sanket1729:
    ACK ca471557a5. How stable are the mutation tests?
  apoelstra:
    ACK ca471557a5

Tree-SHA512: 46a59c90fc25b0c803e96f7c5b98bd39055f7835e45ba137b2a01ad4221a676c54bc228b9ef7663b7300bb4260a6c2c80a0820c4f1bf0987650e1e2bd699f62d
2022-12-30 22:18:31 +00:00
Andrew Poelstra 22820274c1
Merge rust-bitcoin/rust-bitcoin#1496: Verify and fix the `U256::mul_u64` method
cf9733d678 Verify and fix mul_u64 (Tobin C. Harding)

Pull request description:

  Add kani verification for `U256::mul_u64`, doing so uncovered a bug in the current implementation due to overflow. Re-write the `mul_u64` method.

  Fix: #1497

  ## Note

  This PR now _only_ tests `mul_u64`, I will add testing div as a separate PR.

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

Tree-SHA512: c044fb385073bb068412195c079f26057ba560b2a7eca22693c360d324a7c0119cf10da2e70e544c69e05ca8269a81156418d307f4e11d8a4c1c1a7cafa32d0c
2022-12-30 20:43:33 +00:00
Andrew Poelstra 4d7b8cd3b4
Merge rust-bitcoin/rust-bitcoin#1420: Allow dead_code/unused_imports when fuzzing
5a2a37d4be Allow dead_code/unused_imports when fuzzing (Tobin C. Harding)

Pull request description:

  Littering the codebase with `#[cfg(not(fuzzing))]` is a bit messy just to quieten the linter during fuzzing. Instead just globally allow.

  Done while debugging #1409

ACKs for top commit:
  sanket1729:
    ACK 5a2a37d4be
  apoelstra:
    ACK 5a2a37d4be

Tree-SHA512: fb84215a2b00ad6d3321b2781ba285af513ff8fd413c0997045a41c4f23028d2ef0fdf083839289d0c5108c990aa66bdae4430ad3ef32881eac5324b2e881b3b
2022-12-30 14:56:40 +00:00
sanket1729 90bb150422
Merge rust-bitcoin/rust-bitcoin#1512: Make `Witness::tapscript()` return `Script` instead of raw bytes
e0bc50953a Make `Witness::tapscript()` return `Script` instead of raw bytes (Jiri Jakes)

Pull request description:

  Since there is unsized `Script` now, this method can return it.

ACKs for top commit:
  sanket1729:
    utACK e0bc50953a
  tcharding:
    ACK e0bc50953a

Tree-SHA512: 32d4ca14f1b0fc1029f7376b1a43db90332b869a806609c82f660cb2690a4f0e1b91e1799fdac0d43c8a630aed0331f251d4159a662e86e5942c6fb698c42cd2
2022-12-29 23:08:02 -08:00
Tobin C. Harding ca471557a5
locktime: Add mutation testing
Add mutation testing to the `locktime` module and add unit tests to
cover all mutants and ensure they are killed.
2022-12-30 11:13:42 +11:00
Tobin C. Harding 26c0da41b4
locktime: Add inline to public functions
Add `#[inline]` to all public functions/methods excluding error types
and `Display` impls. Error paths do not need to be fast and presumably
`Display` is called on code paths that do IO so this also does not need
to be fast.
2022-12-30 10:19:35 +11:00
Tobin C. Harding cf9733d678 Verify and fix mul_u64
Add kani verification for `U256::mul_u64`, doing so uncovered a bug in
the current implementation due to overflow.

Re-write the `mul_u64` method.

Props to Elichai for the algorithm.

Co-authored-by: Elichai Turkel <elichai.turkel@gmail.com>
2022-12-30 09:39:41 +11:00
sanket1729 c92591e645
Merge rust-bitcoin/rust-bitcoin#1502: Fix bug in `ScriptBuf::extend` for short iterators
920599da94 Add test for previous commit (Martin Habovstiak)
a7f3458c27 Fix bug in `ScriptBuf::extend` for short iterators (Martin Habovstiak)

Pull request description:

  `ScriptBuf::extend` contained an optimization for short scripts that was
  supposed to preallocate the buffer and then fill it. By mistake it
  attempted to fill it from already-exhausted iterator instead of the
  temporary array it drained the items into. This obviously produced
  garbage (empty) values.

  This was not caught by tests because the optimization is only active for
  scripts with known maximum length and the test used `Instructions` which
  doesn't know the maximum length.

ACKs for top commit:
  sanket1729:
    ACK 920599da94 . Tested that the bug is correctly fixed and tested in the new test
  tcharding:
    ACK 920599da94

Tree-SHA512: a80f5f262a840d8e77efd42d63c511224380ee3efa6c31855233e81c90332ac15db228e8d552d039d729d7b642e03c3939c8b6a92d3279001377515acb83abea
2022-12-29 07:52:03 -08:00
Jiri Jakes e0bc50953a Make `Witness::tapscript()` return `Script` instead of raw bytes 2022-12-29 15:22:16 +01:00
Andrew Poelstra 615759a8c2
Merge rust-bitcoin/rust-bitcoin#1495: Introduce mutation testing
8ce928b8e7 Add testing section to readme (Tobin C. Harding)
2e79a0bdc4 Introduce mutation testing (Tobin C. Harding)

Pull request description:

  Introduce mutation testing by way of mutagen [0] (see #1484 for context).

  - Conditionally add the dev-dependency `mutagen` (using `do_mutate` flag)

      This flag is not very well named but `mutagen` and `mutate` are already taken?

  - Mutate all methods of the `U256` struct that do not require additional unit tests.

      Uses `cfg(all(test, do_mutate), mutate)` - I cannot workout why we need to check on `test` as well i.e., I don't understand why we cannot use `cfg(do_mutate, mutate)`?

  With this applied test can be run as usual with a stable toolchain. To run mutagen we use `RUSTFLAGS='--cfg=do_mutate' cargo +nightly mutagen` (doing so runs 29 mutants).

  [0] https://github.com/llogiq/mutagen

ACKs for top commit:
  Kixunil:
    ACK 8ce928b8e7
  apoelstra:
    ACK 8ce928b8e7

Tree-SHA512: 024ba5d2dc983f7cd0444e09ba13280771157204999d2a44502e07a1d6436f571b75003c7cb543c943f17949b848d4070eda4e194bda774a3e41443ff79af0af
2022-12-24 06:02:48 +00:00
Martin Habovstiak 64f7d2549e Fix wrong newtype APIs
This fixes several API bugs:

* Use `TryFrom` instead of `From` for fallible conversions
* Move byte conversion methods from `impl_array_newtype` to
  `impl_bytes_newtype`
* Add missing trait impls like `AsRef`, `Borrow`, their mutable versions
  and infallible conversions from arrays

Closes #1336
2022-12-23 01:28:17 +01:00
Martin Habovstiak 920599da94 Add test for previous commit
If this test is added before the previous commit it will fail. It passes
now demonstrating the bug got fixed.
2022-12-22 23:51:07 +01:00
Martin Habovstiak a7f3458c27 Fix bug in `ScriptBuf::extend` for short iterators
`ScriptBuf::extend` contained an optimization for short scripts that was
supposed to preallocate the buffer and then fill it. By mistake it
attempted to fill it from already-exhausted iterator instead of the
temporary array it drained the items into. This obviously produced
garbage (empty) values.

This was not caught by tests because the optimization is only active for
scripts with known maximum length and the test used `Instructions` which
doesn't know the maximum lenght.
2022-12-22 23:35:03 +01:00
Tobin C. Harding 941083ec4e Remove rand-std dev-dependency from secp256k1
In order to get better test coverage we should not enable the secp26k1
feature "rand-std" in dev-dependencies but instead feature gate tests
that depend on this feature.
2022-12-23 08:33:21 +11:00
Tobin C. Harding f71335f971 Add rand-std feature
Currently we enable "secp256k1/rand-std" in the "rand" feature, this is
incorrect because it means "rand" implies "std" which it does not.

Add a "rand-std" feature that turns on "seck256k1/rand-std" and make the
"rand" feature turn on "seck256k1/rand".
2022-12-23 08:32:56 +11:00
Tobin C. Harding 2e79a0bdc4 Introduce mutation testing
Introduce mutation testing by way of mutagen [0]

- Conditionally add the dev-dependency `mutagen` (using `do_mutate`
flag)

 This flag is not very well named but `mutagen` and `mutate` are already
 taken?

- Mutate all methods of the `U256` struct that do not require additional
  unit tests.

 Uses `cfg(all(test, do_mutate), mutate)` - I cannot workout why we need
 to check on `test` as well i.e., I don't understand why we cannot use
 `cfg(do_mutate, mutate)`?

With this applied test can be run as usual with a stable toolchain. To
run mutagen we use `RUSTFLAGS='--cfg=do_mutate' cargo +nightly mutagen`.

[0] https://github.com/llogiq/mutagen
2022-12-23 08:23:49 +11:00
Martin Habovstiak e428486002 Add `from_bytes(Vec<u8>)` to `ScriptBuf`
This is useful when one already has bytes allocated in a vec that can be
reused.

The change also documents that the mirror method `into_bytes()` doesn't
allocate.
2022-12-22 17:55:19 +01:00
DanGould c4363e5ba1
Deserialize Psbt fields, don't consensus_encode 2022-12-21 12:24:06 -05:00
DanGould c1dd6ad8a2
Serialize Psbt fields, don't consensus_encode them 2022-12-21 12:19:38 -05:00
DanGould 1b7b08aa5d
De/serialize Psbt without consensus traits 2022-12-21 12:01:20 -05:00
Byron Hambly ed1992ffbf
build(deps): bump secp256k1 to 0.25.0 2022-12-20 12:23:15 +02:00
Andrew Poelstra 77aee43685
Merge rust-bitcoin/rust-bitcoin#1485: Add `tapscript_leaf_hash()` to `Script`
bae264d0c2 Add `tapscript_leaf_hash()` to `Script` (Jiri Jakes)

Pull request description:

  Adds convenience method to `Script` for computing leaf hash of tapscript. Closes #1482.

  The little test case is taken from `bip341_tests.json`.

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

Tree-SHA512: fb7a3a552017208decd56ca7d27eab1987a3a92aae5b8620896b3a02986c2fc13043c200ccfbadf9cfdd2d74af38b0bc25936338f55b7d318c1296acc88bf22a
2022-12-19 21:07:59 +00:00
Andrew Poelstra dc91b87990
Merge rust-bitcoin/rust-bitcoin#1477: Patch hashes and update the code
6acf9ac8b8 Patch hashes and update the code (Martin Habovstiak)

Pull request description:

  This patches `bitcoin_hashes` to use the version in the repository and fixes the code after removal of `Deref`.

ACKs for top commit:
  tcharding:
    ACK 6acf9ac8b8
  apoelstra:
    ACK 6acf9ac8b8

Tree-SHA512: b779fa79309f1d648020146b58e96346b67e9f76e29551cbd50251ea6bb7bcfc4c5d42f49cc7ad5660dcd0a5f6306efe96dfcd9530e4b32c62edf4af7076d830
2022-12-19 20:34:20 +00:00
Andrew Poelstra eaee7c52dd
Merge rust-bitcoin/rust-bitcoin#1486: Fix typos in docs
4a6a12011d Fix typos in docs (Daniela Brozzoni)

Pull request description:

  See #828

ACKs for top commit:
  Kixunil:
    ACK 4a6a12011d
  apoelstra:
    ACK 4a6a12011d

Tree-SHA512: 3d3e2c37479986e51595a506c5310a37e51b9a84f9eb2f17c0217430e8150b7a9a7ee8b9c383df6c4ec581a081ea2a722ed4070ff4ede8d777d3bf2a2c19f15e
2022-12-19 20:24:09 +00:00
Daniela Brozzoni 4a6a12011d
Fix typos in docs
See #828
2022-12-19 09:32:52 +01:00
Jiri Jakes bae264d0c2 Add `tapscript_leaf_hash()` to `Script` 2022-12-19 08:35:35 +01:00
Martin Habovstiak 6acf9ac8b8 Patch hashes and update the code
This patches `bitcoin_hashes` to use the version in the repository and
fixes the code after removal of `Deref`.

This also turns off `AS_DEPENDENCY` check with the intention to refactor
it later.
2022-12-18 14:33:55 +01:00
Casey Rodarmor 67ca3463c0 Mention Script::is_v1_p2tr above Witness::tapscript 2022-12-17 16:37:01 -08:00
Andrew Poelstra 0203107360
Merge rust-bitcoin/rust-bitcoin#1475: add some documentation clarifying the locktime ordering shenanigans in #1330
02c1cd6291 add some documentation clarifying the locktime ordering shenanigans in #1330 (Andrew Poelstra)

Pull request description:

  Updates the CHANGELOG and also the doccomment on `Transaction`.

ACKs for top commit:
  tcharding:
    ACK 02c1cd6291
  Kixunil:
    ACK 02c1cd6291
  sanket1729:
    ACK 02c1cd6291

Tree-SHA512: e2d23a90fb1e53758449fe49a3db7ae1497a260ce7efcade4b50265fa70840db273609019590d9d0a69e1272607a6bcf37924b805b4f09909487eb0c3b91a3cd
2022-12-16 14:41:00 +00:00
Andrew Poelstra 02c1cd6291
add some documentation clarifying the locktime ordering shenanigans in #1330 2022-12-15 23:12:03 +00:00
Andrew Poelstra c657a1be3c
Merge rust-bitcoin/rust-bitcoin#1467: Add weight utilities to `TxIn` and `TxOut`
6d51e9255b Add weight utilities to `TxIn` and `TxOut` (Daniela Brozzoni)

Pull request description:

  - Add `segwit_weight` and `legacy_weight` methods to `TxIn`
  - Add `weight` method to `TxOut`

ACKs for top commit:
  danielabrozzoni:
    > ACK [6d51e92](6d51e9255b)
  apoelstra:
    ACK 6d51e9255b
  Kixunil:
    ACK 6d51e9255b

Tree-SHA512: 217eae49b5f6e8149af251fb82682aed34e0003342d19ec66aa0f66b8044d50c18d1e3e2d58068e4d2572b1af8bbc3403bfd5447662b45bc4f1e0e7f0672964f
2022-12-15 20:21:00 +00:00
Daniela Brozzoni 6d51e9255b
Add weight utilities to `TxIn` and `TxOut`
- Add `segwit_weight` and `legacy_weight` methods to `TxIn`
- Add `weight` method to `TxOut`
2022-12-15 09:20:56 +01:00
Martin Habovstiak 8e428562cb Implemented unsized `Script`
This renames `Script` to `ScriptBuf` and adds unsized `Script` modeled
after `PathBuf`/`Path`. The change cleans up the API a bit, especially
all functions that previously accepted `&Script` now accept truly
borrowed version. Some functions that perviously accepted `&[u8]` can
now accept `&Script` because constructing it is no loger costly.
2022-12-14 23:21:27 +01:00
Andrew Poelstra f231617103
Merge rust-bitcoin/rust-bitcoin#1330: Remove `PackedLockTime` in favor of `absolute::LockTime`
1b15a13e5a run cargo clippy and fmt (Andrew Poelstra)
f2a5596899 examples: clean up taproot PSBT example locktime handling (Andrew Poelstra)
821842e1a1 drop Ord on absolute::LockTime; add Ord to Transaction (Andrew Poelstra)
5b7d801ee6 remove PackedLockTime type (Andrew Poelstra)
4dee116b8a delete PackedLockTime by aliasing it to LockTime (Andrew Poelstra)
fa81568fb6 locktime: add `FromHexStr` impl for `LockTime` (Andrew Poelstra)
74ff4946e4 locktime: unify serde impls (Andrew Poelstra)

Pull request description:

  This is potentially a controversial PR, but hear me out. My argument is that right now `absolute::LockTime` and `PackedLockTime` are basically identical types; they can be converted between each other with `From` and they have exactly the same semantics except that `PackedLockTime` has `Ord` on it. This makes it very confusing to tell which one should be used, for example in PSBT2 where there are now extra locktime-related fields.

  The motivation for having `PackedLockTime` independent of `LockTime` are:
  * `PackedLockTime` is theoretically more efficient because you don't need to unpack the field, don't need to store a enum discriminate, etc. I don't buy this. If you are trying to save individual bytes in your transaction-parsing there are lots of places you'd look before messing around with locktimes, so we shouldn't privilege this specific thing.
  * `PackedLockTIme` has an `Ord` impl, and removing that will have a cascading effect on transactions, blocks, etc., preventing them from being held in `BTreeMaps` etc. **My proposal**, implemented here, is to just manually impl `Ord` on `Transaction` and don't impl it on `LockTime`.

  I recall some argument that we need to be able to sort miniscripts, and miniscripts might have locktimes in them, but I think this is wrong -- miniscripts always have explicitly either a `Height` or a `Time`, and there is no problem ordering these.

  Closes #1455

ACKs for top commit:
  sanket1729:
    code review ACK 1b15a13e5a
  tcharding:
    ACK 1b15a13e5a

Tree-SHA512: d9776f14560c3822980e8fff8978bf8ca753035152f957a84af25d063c66e4e9df3d5cf98af38d6cb59cc52ba407282f722925b1ca670ae623ac91add3149d2f
2022-12-14 18:21:07 +00:00
Andrew Poelstra d581207cb8
Merge rust-bitcoin/rust-bitcoin#1450: hashes: Do not implement `Deref`
b7a84d0c68 hashes: Do not implement Deref (Tobin C. Harding)

Pull request description:

  Currently we implement `Deref` for hashes. From the docs [0]

   > Deref should only be implemented for smart pointers to avoid confusion

  Furthermore because we implement `Deref` as well as implement `internals::hex::display::DisplayHex` for slices hashes get coerced into slices and `to_lower_hex_string` can be called on them, this is incorrect because `DisplayHex` does not account for hashes that display backwards so we end up with the wrong string.

  This is an API breaking change, and I have not built any other crates in our stack to check if anything breaks.

  [0] https://doc.rust-lang.org/std/ops/trait.Deref.html

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

Tree-SHA512: 573169095dd9f9032e3f685e3b0af3b569f1cb5368e1b2f37940504fd887592f46488abcfbe84107baf9c5453c7db47bc342a6bc273ff98cb0570ffacb549c21
2022-12-13 20:59:41 +00:00
Andrew Poelstra 1b15a13e5a
run cargo clippy and fmt 2022-12-13 14:52:43 +00:00
Andrew Poelstra f2a5596899
examples: clean up taproot PSBT example locktime handling
This still has the line

    let lock_time = absolute::LockTime::from_height(psbt.unsigned_tx.lock_time.to_consensus_u32() + lock_time_delta).unwrap();

I'm unsure whether this "adding height to a locktime" concept is a
meaningful thing or just the sort of thing that shows up in example
code. Maybe we should have first-class support for it.

Note that the line, as written, depends on the fact that the original
locktime was a small blockheight. A proper function for this would
handle the exceptional case gracefully.
2022-12-13 14:52:43 +00:00
Andrew Poelstra 821842e1a1
drop Ord on absolute::LockTime; add Ord to Transaction 2022-12-13 14:52:36 +00:00
Tobin C. Harding b7a84d0c68 hashes: Do not implement Deref
Currently we implement `Deref` for hashes. From the docs [0]

 > Deref should only be implemented for smart pointers to avoid confusion

Furthermore because we implement `Deref` as well as implement
`internals::hex::display::DisplayHex` for slices hashes get coerced into
slices and `to_lower_hex_string` can be called on them, this is
incorrect because `DisplayHex` does not account for hashes that display
backwards so we end up with the wrong string.

[0] https://doc.rust-lang.org/std/ops/trait.Deref.html
2022-12-12 12:05:54 +11:00