Commit Graph

2855 Commits

Author SHA1 Message Date
Andrew Poelstra 1b8f52a804
Merge rust-bitcoin/rust-bitcoin#1567: Use `sha256d::Hash` type for sighash encoding
49e8b8da32 Use write_all for sighash encoding (Tobin C. Harding)

Pull request description:

  From BIP143:

  > If sighash type is SINGLE and the input index is smaller than the number of outputs, hashOutputs is the double SHA256 of the output amount with scriptPubKey of the same index as the input;

  Currently we are using a `Sighash` which wraps double sha256 so while technically correct this means we are relying on `Sighash` to implement `Encodable`. We can remove this requirement by directly using the `sha256d::Hash` type to hash the outputs data.

  Fix: #1549

ACKs for top commit:
  Kixunil:
    ACK 49e8b8da32
  apoelstra:
    ACK 49e8b8da32

Tree-SHA512: 8dd0037245a7cf180ba8a6eceeadad912d4adc14fc3f49df9008856de262624666d7d575195eea4868b2a5252dc565590e6be78471053b5e6367f3d2363310e8
2023-01-24 14:52:25 +00:00
Andrew Poelstra f6d983b2ef
Merge rust-bitcoin/rust-bitcoin#1532: Improve Psbt error handling
e7bbfd3913 Improve Psbt error handling (DanGould)

Pull request description:

  ## Separate `encode::Error` and `psbt::Error` recursive dependency

  This initial work attempts to fix #837's first 2 points

  > - The current psbt::serialize::Deserialize has an error type of consensus::encode::Error. I think we should cleanly separate consensus encoding errors from application-level encoding errors like psbt.
  > - There is a recursive dependence between encode::Error and psbt::Error which would need to be cleanly dissected and separated so that there is no dependence or only one-way dependence.

  ## Better `ParseError(String)` types

  arturomf94 how compatible do your #1310 changes look to address #837's third point with this design?

  > - There are a lot ParseError(String) messages that could use a better type to downflow the information.

  I think your prior art would completely address this issue now.

  ## On handling `io::Error` with an associated error

  `encode::Error` has an `Io` variant. now that `Psbt::deserialize` returns `psbt::Error` and produces an `io::Error`, we need an `Io` variant on `psbt::Error`. Except that doing so breaks  `#[derive(Eq)]` and lots of tests for `psbt::Error`.

  Kixunil, I'm trying to understand your feedback regarding a solution to this problem.

  > I believe that the best error untangling would be to make decodable error associated.

  > I meant having associated `Error` type at `Decodable` trait. Encoding should only fail if the writer fails so we should have `io::Error` there (at least until we have something like `genio`).
  >
  > > [it] is a problem to instantiate consensus::encode::Error in [the psbt] module for `io::Error`?
  >
  > It certainly does look strange. Maybe we should have this shared type:
  >
  > ```rust
  > /// Error used when reading or decoding fails.
  > pub enum ReadError<Io, Decode> {
  >     /// Reading failed
  >     Io(Io),
  >     /// Decoding failed
  >     Decode(Decode), // consensus and PSBT error here
  > }
  > ```
  >
  > However this one will be annoying to use with `?` :( We could have `ResultExt` to provide `decode()` and `io()` methods to make it easier.
  >
  > If that's not acceptable then I think deduplicated IO error is better.

  Kixunil didn't we just get rid of Psbt as `Decodable`? Would this make more sense to have as an error associated with `Deserialize`? Or did we do the opposite of what we should have by making Psbt only `Serialize`/`Deserialize` because of #934, where only consensus objects are allowed to be `Decodable`? I wonder if we prioritized that strict categorization and are stuck with worth machinery because of it. My goal with #988 was to get to a point where we could address #837 and ultimately implement PSBTv2.

ACKs for top commit:
  tcharding:
    ACK e7bbfd3913
  apoelstra:
    ACK e7bbfd3913

Tree-SHA512: 32975594fde42727ea9030f46570a1403ae1a108570ab115519ebeddc28938f141e2134b04d6b29ce94817ed776c13815dea5647c463e4a13b47ba55f4e7858a
2023-01-24 14:18:25 +00:00
Andrew Poelstra d8d34116ad
Merge rust-bitcoin/rust-bitcoin#1564: Address validity invariant cleanups
44d3ec487d Rename Payload::as_bytes to inner_prog_as_bytes (sanket1729)
a446df583c Make Payload non-exhaustive (sanket1729)
6ebc9de252 Introduce WitnessProgram struct and cleanup Address validity invariants (sanket1729)
41652caf05 Introduce is_spend_standard method (sanket1729)

Pull request description:

  Fixes #1561.

  Highlights:

  - Segwitv0 programs with lengths apart from 20 or 32 are invalid `Address` struct. Such Addresses are useless and we should not parse/create them.
  - Renamed `is_standard` to `is_spend_standard`.

ACKs for top commit:
  apoelstra:
    ACK 44d3ec487d
  tcharding:
    ACK 44d3ec487d

Tree-SHA512: 1ee36f7ea25c65619ddf7d643d025690096876843dbe6fbdf877ce23e88049d79b0bbd78cee6cf4b415bca028b3634bb70c9f52d1098bd90558e6ba7f8731332
2023-01-24 13:25:49 +00:00
Andrew Poelstra c4b8d452da
Merge rust-bitcoin/rust-bitcoin#1544: amount parses 0 msats
0cf6da4f03 amount parses 0 msats (bnabi)

Pull request description:

  Closes #1529

  0 msats should be parasable.

ACKs for top commit:
  Kixunil:
    ACK 0cf6da4f03
  tcharding:
    ACK 0cf6da4f03
  apoelstra:
    ACK 0cf6da4f03

Tree-SHA512: aeb15ed46f21b3e75bbd80094a103900ad0f42995abdd32476db771c623aea6e3c557a4180f1d9a92d0ad62e703b9ad838d9be1b43d61624dd6e6bce31a8b81b
2023-01-23 22:12:24 +00:00
sanket1729 44d3ec487d Rename Payload::as_bytes to inner_prog_as_bytes
Also make it private
2023-01-23 13:45:46 -08:00
sanket1729 a446df583c Make Payload non-exhaustive
It is unlikely that we will see another segwit type fork. But we never
know, best to mark Payload as [non-exhaustive]
2023-01-23 13:45:46 -08:00
sanket1729 6ebc9de252 Introduce WitnessProgram struct and cleanup Address validity invariants
Addresses with Segwitv0 not having len 20/32 are invalid and cannot be
constructed. Also cleans up a API bug in
ScriptBuf::new_witness_prog(ver, prog) allowing prog of invalid lenghts.
2023-01-23 13:45:46 -08:00
Andrew Poelstra 4b02d90ce4
Merge rust-bitcoin/rust-bitcoin#1578: Implement `Debug` for generic `Address<V: NetworkValidation>`
ebfbe74243 Implement `Debug` for generic `Address<V: NetworkValidation>` (Jiri Jakes)

Pull request description:

  Previously `Debug` was implemented for both `Address<NetworkChecked>` and `Address<NetworkUnchecked>`, but not for cases when the `NetworkValidation` parameter was generic. This change adds this ability. Based on Kixunil's tip.

  With previous implementation, the `test_address_debug()` resulted in error:

  ![image](https://user-images.githubusercontent.com/1381856/213907042-f1b27f41-fa46-4fa0-b816-cc4df53f5d29.png)

  The added `Debug` on `NetworkChecked` and `NetworkUnchecked` are required by compiler.

  ---

  While dealing with derives and impls, I also attempted to turn all the derives on `Address` into manual impls (see Kixunil's suggestion in https://github.com/rust-bitcoin/rust-bitcoin/pull/1489#discussion_r1052448057). The motivation behind this was the possibility to remove derives on `NetworkChecked` and `NetworkUnchecked`, too. However, even with manual impls, all the traits on `NetworkChecked` and `NetworkUnchecked` were still required by compiler in this sort of situations (see also the rest of the same discussion linked above). I do not fully understand why, perhaps limitation of this way of sealing traits?

  It can be demonstrated by removing `Debug` derivation on `NetworkUnchecked` and `NetworkChecked` in this PR and running `test_address_debug()`.

  Therefore, if we want to allow users of the library to define types generic in `NetworkValidation` and at the same time derive impls, it seems to me that `NetworkChecked` and `NetworkUnchecked` will have to have the same set of impls as `Address` itself.

ACKs for top commit:
  Kixunil:
    ACK ebfbe74243
  tcharding:
    ACK ebfbe74243
  apoelstra:
    ACK ebfbe74243

Tree-SHA512: 87f3fa4539602f31bf4513a29543b04e943c3899d8ece36d0d905c3b5a2d76e29eb86242694b5c494faa5e54bb8f69f5048849916c6438ddd35030368f710353
2023-01-23 14:49:22 +00:00
Andrew Poelstra 47027081a5
Merge rust-bitcoin/rust-bitcoin#1576: bip158: Replace usage of HashSet with BTreeSet
6e56feed57 bip158: Replace usage of HashSet with BTreeSet (Tobin C. Harding)

Pull request description:

  The `bip158` module uses a `HashSet` and in order to do so requires the `hashbrown` dependency for "no-std" builds.

  We can replace the usage of `HashSet` with a `BTreeSet` in `bip158` and remove the `hashbrown` dependency entirely.

  This patch makes no claims about performance cost or benefit of this change. The patch also makes no claims about the validity of the current `HashSet` usage.

  The `hashbrown` dependency and `HashSet` usage can be trivially added back in if someone comes up with perf data to back it up.

ACKs for top commit:
  Kixunil:
    ACK 6e56feed57
  apoelstra:
    ACK 6e56feed57

Tree-SHA512: 6e8d6af7ccf22031a22ea19b731fbe5f6cbfdfb1e510119a7dbdd2e8521eeb3b0d2c7b9e000641deae8960287099dbf58515341720cf7c41d1fcab08308b0b74
2023-01-23 13:38:41 +00:00
Tobin C. Harding 1d3d5a9c5b
Take Into<secp256k1::PublicKey> in PublicKey constructors
We can make the API more ergonomic by taking a generic argument that
implements `Into<secp256k1::PublicKey>` in the `bitcoin::PublicKey`
constructors.

The only thing than this is useful for is passing in `KeyPair` and the
`From` implementation already exists. Add a unit test to verify.

Fix: #1453
2023-01-23 14:46:53 +11:00
Tobin C. Harding b13a76407b
keys: Clean up test imports
Clean up the test import statements in the `keys` module. Done in
preparation for adding a unit test.
2023-01-23 14:46:53 +11:00
Tobin C. Harding 6d99d3c061
Use ignore to stop rustdoc code from being built
Currently we have an attempted tag ```compile_fail that seems to be
aiming at allowing code that does not build to exist in rustdoc. This is
causing an error when running tests.

No clue how this made it through CI.

Use ```ignore to prevent rustdoc code from being built.
2023-01-23 13:26:15 +11:00
Tobin C. Harding 8c0e5213d3
Delegate debug for ScriptBuf to Script
Currently the derived implementation of `Debug` for `ScriptBuf` prints
the inner vector of u8s as integers, this is ugly and hard to read. The
`Script` implementation of `Debug` prints the script opcodes and data as
hex, we can just delegate to it.

With this applied we get debug output of form:

    Script(OP_DUP OP_HASH160 OP_PUSHBYTES_20 \
    3bde42dbee7e4dbe6a21b2d50ce2f0167faa8159 OP_EQUALVERIFY OP_CHECKSIG)

Fix: #1516
2023-01-23 12:49:11 +11:00
Tobin C. Harding 097e4e9c7f
Fix license on bip158 module
When we introduced the SPDX license blurb in [0] we incorrectly gave
attribution to Andrew when the original file author had the attribution
as "the rust-bitcoin developers". The original author [1] was Tamas
Blummer and he copied this code from code he wrote and explicitly
re-licenses it. In order to make the re-licensing comment a little
clearer and fix the mis-attribution use Tamas' name in the attribution.

[0] commit: `91ff2f628ce7db732d234a812e29fa8508f501a1 Introduce SPDX license identifiers`
[1] commit: `c93a70487f81a93c7d479ae046c75590d9fb7733 Add client side block filter (BIP158) (#281)`
2023-01-23 10:47:50 +11:00
Tobin C. Harding a762a89b48
Add documentation to Sequence::is_final
The term "final" is an archaic Bitcoin term however it is well used, it
exists in Bitcoin Core code as well as in various bips. To help folks
new to Bitcoin add documentation to the `is_final` method including
historical notes.
2023-01-23 09:18:23 +11:00
Tobin C. Harding 6e56feed57
bip158: Replace usage of HashSet with BTreeSet
The `bip158` module uses a `HashSet` and in order to do so requires the
`hashbrown` dependency for "no-std" builds.

We can replace the usage of `HashSet` with a `BTreeSet` in `bip158` and
remove the `hashbrown` dependency entirely.

This patch makes no claims about performance cost or benefit of this
change. The patch also makes no claims about the validity of the current
`HashSet` usage.

The `hashbrown` dependency and `HashSet` usage can be trivially added
back in if someone comes up with perf data to back it up.
2023-01-23 08:38:08 +11:00
Jiri Jakes ebfbe74243 Implement `Debug` for generic `Address<V: NetworkValidation>`
Previously `Debug` was implemented for both `Address<NetworkChecked>`
and `Address<NetworkUnchecked>`, but not for cases when the
`NetworkValidation` parameter was generic. This change adds this
ability.
2023-01-22 16:23:10 +08:00
bnabi 0cf6da4f03 amount parses 0 msats 2023-01-22 12:08:25 +05:30
Tobin C. Harding b1490a26ea
Move enables_absolute_lock_time method
In preparation for deprecating the `is_final` method; move the
`enables_absolute_lock_time` method to be directly above the `is_final`
method.

Refactor only, no logic changes.
2023-01-22 17:04:58 +11:00
Tobin C. Harding e079524b2a
Remove hashes::hex::HexWriter
The `HexWriter` is not used any more since we added the new hex code in
internals for fast hex encoding.

While we are removing the benches for `HexWriter` also remove the last
remaining bench for writing using `Display` because this is not the
correct place for that code - its trivial to re add later in the correct
module.
2023-01-22 12:05:08 +11:00
Tobin C. Harding 49e8b8da32
Use write_all for sighash encoding
`Sighash` does not need to implement `Encodable` because it is
claimed (I don't know exactly myself) that `Sighash` is never consensus
encode in Bitcoin.

We are currently relying on `Sighash` to implement `Encodable` when
encoding creating the segwit v0 sighash for a single input.

For reference, from BIP143:

 If sighash type is SINGLE and the input index is smaller than the
 number of outputs, hashOutputs is the double SHA256 of the output
 amount with scriptPubKey of the same index as the input;

We can use `write_all` directly to write the hashed bytes and remove the
implementation of `Encodable` from the `Sighash` type.

While we are at it, use `write_all` to write the zero hash also to make
the code more uniform and understandable.

Fix: #1549
2023-01-21 10:56:59 +11:00
Tobin C. Harding 70bff0de8d
ci: Remove incorrect code comment and cargo clean
Recently while trying to fix CI I (Tobin) added a call to `cargo clean`
and a code comment justifying it. This was merged because while not
incorrect it is redundant since calling `cargo` with different
`RUSTFLAGS` triggers a rebuild.
2023-01-21 09:37:16 +11:00
sanket1729 41652caf05 Introduce is_spend_standard method 2023-01-20 07:31:21 -08:00
sanket1729 d66ee48482
Merge rust-bitcoin/rust-bitcoin#1566: Fix ASAN in CI
53ee42d7c1 Fix ASAN in CI (Tobin C. Harding)

Pull request description:

  When we merged `hashes` into `rust-bitcoin` we ran the test script a couple of times using `./hashes/contrib/test.sh` this causes the `cargo` commands in the CI script to be run from the crate root which is not what we want.

  This showed up recently because `cargo test` was getting run in `bitcoin` after building with address/memory sanitazation configured into the build.

  While we are at it run `cargo clean` after the last sanitizer build just to future proof the CI script in case we later accidentally re-use the same build with plain old `cargo` (without `-Zbuild-std etc.`).

ACKs for top commit:
  apoelstra:
    ACK 53ee42d7c1
  sanket1729:
    ACK 53ee42d7c1

Tree-SHA512: 3d0b9aa35be6290808e2d23144e4dce467ab433862b375e1f164dc41261919e8e27e4cab117bbe9bfe18814ec7bf468e1c03674d20e9f36b5319e986fc329e89
2023-01-20 07:12:34 -08:00
Tobin C. Harding 53ee42d7c1
Fix ASAN in CI
When we merged `hashes` into `rust-bitcoin` we ran the test script a
couple of times using `./hashes/contrib/test.sh` this causes the `cargo`
commands in the CI script to be run from the crate root which is not
what we want.

This showed up recently because `cargo test` was getting run in
`bitcoin` after building with address/memory sanitazation configured
into the build.

While we are at it run `cargo clean` after the last sanitizer build just
to future proof the CI script in case we later accidentally re-use the same
build with plain old `cargo` (without `-Zbuild-std etc.`).
2023-01-20 15:47:43 +11:00
DanGould e7bbfd3913
Improve Psbt error handling
Remove recursive dependence between encode::Error and psbt::Error.
Separate consensus encoding errors from Psbt application errors.
2023-01-17 16:43:39 -05:00
Andrew Poelstra 649bf023af
Merge rust-bitcoin/rust-bitcoin#1537: Export the DisplayHex trait from within prelude
70fe07f1ce Export the DisplayHex trait from within prelude (Tobin C. Harding)

Pull request description:

  We use `internals::hex::display::DisplayHex` in many places, we can improve ergonomics of the `internals` crate by re-exporting it from the `prelude` module.

ACKs for top commit:
  Kixunil:
    ACK 70fe07f1ce
  apoelstra:
    ACK 70fe07f1ce

Tree-SHA512: 96a89135cb0b829b7b5926a3b344f78e178b5b48e772a69da5133fab6d2e14e7b7bbaa56b7a417a5c1a64337546a1c7bac32307d3a1f27aa199ed61f590902bf
2023-01-16 15:21:33 +00:00
Tobin C. Harding 41f2dcf6ae
Improve test coverage for docs build
Currently the docs build commands in `hashes` and `bitcoin` differ, they
should be the same.

Add a command `cargo doc` to improve coverage e.g., recently we botched
the feature guarding but since CI only runs `cargo rustdoc` with custom
compiler conditional set we didn't catch it.

Run docs in CI using nightly and stable toolchains as required.
2023-01-16 13:56:56 +11:00
Tobin C. Harding b4c14a4b7c
hashes: Use automatic link
Cargo emits various warnings when building the docs:

 warning: this URL is not a hyperlink

As suggested, add angle brackets to create automatic links.
2023-01-16 13:56:27 +11:00
Tobin C. Harding 96e8a080d1
ci: Remove redundant || exit
We already use `set -ex`, adding an explicit exit on command fail is
redundant.
2023-01-16 13:56:26 +11:00
Andrew Poelstra 006fe3c223
Merge rust-bitcoin/rust-bitcoin#1539: Add a rustdoc test to Denomination
a7dd4b5ab0 Add a rustdoc test to Denomination (Tobin C. Harding)

Pull request description:

  Add a rustdoc test to the `Denomination` type to show basic usage.

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

Tree-SHA512: 08f15c4641e70f043276b873e60fcf0e195fe50b6c5c18a245d2d609f4a4a4badc291aae1be532fc4890a91b2057cd308c86d0d3b85770b600a07499303a7bc4
2023-01-15 23:00:51 +00:00
Andrew Poelstra 84a120de26
Merge rust-bitcoin/rust-bitcoin#1510: Add more mutation testing to the `pow` module
308c727c82 pow: Add more mutation testing (Tobin C. Harding)
948b04927d Remove unnecessary parenthesis (Tobin C. Harding)
c3021d852a Fix typo in code comment (Tobin C. Harding)

Pull request description:

  Add more mutation testing to the `pow` module.

  The first two patches are preparatory clean up.

  All functions/methods that are non-trivial are tested excluding:
  - `to_compact_lossy`
  - `from_compact`
  - `fmt_decimal`

  I think its ok to not test `fmt_decimal`. The compact format ones require a bit of work to do but should be done at some stage.

ACKs for top commit:
  Kixunil:
    ACK 308c727c82
  apoelstra:
    ACK 308c727c82

Tree-SHA512: 1bd8df00521676c2193f482d6e3b557c727e7807211acc11c311aa715ebe94ae957352897c084474b22dc33c589382ef557c33f95c467929b7c1659128fdced0
2023-01-15 22:36:35 +00:00
Andrew Poelstra 57098872ae
Merge rust-bitcoin/rust-bitcoin#1545: Add CI script for the `internals` crate
f4e7def72f internals: Add CI test script (Tobin C. Harding)
db6b3f1df5 Fix method names in rustdoc (Tobin C. Harding)
7593f1f334 Use set -ex (Tobin C. Harding)

Pull request description:

  Quick merge this while noone's looking ... we forgot to test the new `internals` crate in CI.

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

Tree-SHA512: c603d2308911210eeed17f128ef684ea161e33f5749d3d5567bb15ac4a01695eb07718873e53824bb21159427b453b04265db6728b9839a13092a6817ac9c4d8
2023-01-15 20:25:21 +00:00
Tobin C. Harding 308c727c82
pow: Add more mutation testing
Recently we introduced some mutation testing to the `pow` module but
testing is never done - add more `mutate` attributes and add unit tests
to ensure all mutants are killed.

Of note, the `from_compact` and `to_compact_lossy` functions are not
done, doing so results in a bunch of surviving mutants.
2023-01-13 08:23:14 +11:00
Tobin C. Harding 948b04927d
Remove unnecessary parenthesis
Plain integer does not require parenthesis, remove them.
2023-01-13 08:22:45 +11:00
Tobin C. Harding c3021d852a
Fix typo in code comment
Fix a trivial grammatical mistake in code comment.
2023-01-13 08:21:34 +11:00
Tobin C. Harding f4e7def72f
internals: Add CI test script
We do not currently run the `internals` crate tests in CI. Bad
rust-bitcoin developers, no biscuit.
2023-01-13 07:59:38 +11:00
Tobin C. Harding db6b3f1df5
Fix method names in rustdoc
We have a few incorrect method names in the rustdocs for `internals`,
fix them up.
2023-01-13 07:44:51 +11:00
Tobin C. Harding 7593f1f334
Use set -ex
Use the more typical form to enable bash features.

Refactor only, no logic changes.
2023-01-13 07:32:30 +11:00
Tobin C. Harding a7dd4b5ab0
Add a rustdoc test to Denomination
Add a rustdoc test to the `Denomination` type to show basic usage.
2023-01-13 07:19:30 +11:00
Andrew Poelstra e36ae3a229
Merge rust-bitcoin/rust-bitcoin#1489: Use marker type to enforce validation of `Address`'s network
bef7c6e687 Use marker type to enforce validation of `Address`'s network (Jiri Jakes)

Pull request description:

  Adds marker type `NetworkValidation` to `Address` to help compiler enforce network validation. Inspired by Martin's suggestion. Closes #1460.

  Open questions:

   1. Compilation fails with serde, which uses `Address:from_str` via macro `serde_string_impl!(Address, "a Bitcoin address");`. I don't think there is much we can do, so unless somebody has a better idea how to combine serde and network validation, I would just demacroed the macro for `Address` and add `unsafe_mark_network_valid` into it.
   2. Would someone prefer wrapping the validation types by `mod validation`? As they are now, they live in `address` namespace so I don't think mod is necessary.
   3. Almost all methods that used to be on `Address` are now on `Address<NetworkValid>` except one (`address_type`) that needs to be called on both and a few that are only on `Address<NetworkUnchecked>` (mainly `is_valid_for_network`). Some methods (e. g. `to_qr_uri`, `is_standard` and perhaps others) could be, theoretically, called on both valid and unchecked. I think we should encourage validating the network ASAP, so I would leave them on NetworkValid only, but I can move them if others have different opinion.
   4. Should `NetworkValid` and `NetworkUnchecked` enums have some trait impls derived? The `PartialEq` was necessary for tests (I think `assert_eq` required it) but I am not sure whether some other would be good to have. The enums are only used as types so I guess it's not necessary, but also I do not fully understand why the `PartialEq` was needed.

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

Tree-SHA512: 8d18b25e62c594468625b39ab174b27ae04b98082cd6011283fe657e868a525f0e4cb40d0f68aff44ad145582e9f3c6387bd2077c5c1f2857d4032674121c31f
2023-01-11 20:13:41 +00:00
Jiri Jakes bef7c6e687 Use marker type to enforce validation of `Address`'s network
Parsing addresses from strings required a subsequent validation of
network of the parsed address. However, this validation was not
enforced by compiler, one had to remember to perform it.

This change adds a marker type to `Address` that will assist the
compiler in enforcing this validation.
2023-01-11 19:27:10 +08:00
sanket1729 ac6340943c
Merge rust-bitcoin/rust-bitcoin#1538: A crash course in rust perf (#988 nits)
1a409ecc8e Use &[u8] instead of Cursor as Read (DanGould)
81ca10701a Use as_ref() instead of costly clone() (DanGould)
debcce6a03 Improve magic bytes push performance (DanGould)

Pull request description:

  A follow up to address [nits in  #988](https://github.com/rust-bitcoin/rust-bitcoin/pull/988#pullrequestreview-1233687052)

ACKs for top commit:
  tcharding:
    ACK 1a409ecc8e
  elichai:
    ACK 1a409ecc8e
  sanket1729:
    utACK 1a409ecc8e

Tree-SHA512: bc6c538d8f92b0efa8063ba33bb47f0c9dc2f99eb5db267b66aa01fc70fd634695a0cd9c27fb0f33e3498628729a8e0f622d9876dc1c83fab4a1e46484d425b2
2023-01-11 02:38:33 -08:00
sanket1729 8df00339ec
Merge rust-bitcoin/rust-bitcoin#1479: Unify `TapLeafHash` and `TapBranchHash` into `TapNodeHash` while tree construction
f39cd88f5f Use TapNodeHash in NodeInfo (sanket1729)
5ff2635585 Rename TapBranchHash -> TapNodeHash (sanket1729)

Pull request description:

  This does not completely fix #1393 but is a simple starter. Unfortunately, we still need hash_new_type for `TapNodeHash` because is exported as the Merkle root in taproot psbt. This requires serialization and display/from_str methods.

  This also adds a method 1) `TapNodeHash::from_script` to deal with single leaf case cleanly. As a nice side effect, this removes the ugly uses of `sha256::Hash` that I had used to represent both `TapLeafHash` and `TapBranchHash`

  Does not fix
  1) Exporting `TapTweakHash`. This requires some changes to macros we use.
  2) Safer usage of `TapNodeHash` by not making it `hash_new_type` and having guarded constructors. As mentioned above, this is required for psbts and sharing Merkle roots. Perhaps, we might need a new type while constructing trees that is not `hash_new_type`, and convert it to another type that represents the final Merkle root.

  My overall feeling is that this is best addressed with more examples than changing and complicating existing code.

  I don't feel strongly about the rename, so can go back if `TapBranchHash` instead of `TapNodeHash`.

ACKs for top commit:
  Kixunil:
    ACK f39cd88f5f
  tcharding:
    ACK f39cd88f5f

Tree-SHA512: 55310b89442ac1f73cce8a7202bbed6ac5a0bbefec0a938fe6065e65c65fe1d34c9e2b5e30ce56ae2bbd0cc9e4caa1363dbd03e9feb7929acb47bbac9f2a4191
2023-01-11 02:35:58 -08:00
DanGould 1a409ecc8e
Use &[u8] instead of Cursor as Read 2023-01-09 23:21:20 -05:00
DanGould 81ca10701a
Use as_ref() instead of costly clone() 2023-01-09 23:17:04 -05:00
DanGould debcce6a03
Improve magic bytes push performance 2023-01-09 23:10:07 -05:00
Tobin C. Harding 70fe07f1ce
Export the DisplayHex trait from within prelude
We use `internals::hex::display::DisplayHex` in many places, we can
improve ergonomics of the `internals` crate by re-exporting it from the
`prelude` module.
2023-01-10 09:56:41 +11:00
sanket1729 f39cd88f5f Use TapNodeHash in NodeInfo
This cleans up some ugly code where we had to use sha256::Hash for
combining TapLeafHash and TapBranchHash
2023-01-09 13:01:44 -08:00
sanket1729 5ff2635585 Rename TapBranchHash -> TapNodeHash 2023-01-09 12:42:46 -08:00