Commit Graph

1442 Commits

Author SHA1 Message Date
Dr Maxim Orlovsky 6add0dd9dc
Track information about hidden leaves in taproot NodeInfo 2022-04-05 22:16:27 +02:00
0xb10c 548725c5fb
test: reject message (de)serialization
This adds tests for the previously untested reject message
(de)serialization. The two reject messages were received from an
older Bitcoin Core peer that still sends reject messages.
2022-04-05 08:35:11 +02:00
0xb10c fc572aba86
fix: use var_str in 'reject' msgs
CommandString is (de)serialized as 12 bytes. However, BIP-61 defines
the 'response-to-msg' (message that triggered the reject) field
to be a var_str [1].

[1]: https://github.com/bitcoin/bips/blob/master/bip-0061.mediawiki#common-payload
2022-04-05 08:30:16 +02:00
Tobin Harding 29843c41ef Allow deprecated function call
We have a deprecated function call because of the MSRV, tell clippy to
ignore it.
2022-04-04 18:28:09 +10:00
Jeremy Rubin 7969b7a43e Make TaprooBuilder::finalize able to return keyspend only 2022-04-03 16:24:56 -04:00
sanket1729 cb4d34fd40
Merge rust-bitcoin/rust-bitcoin#932: Derive Eq for PSBT types
603e75eb77 Derive Eq for PSBT types (Dr Maxim Orlovsky)

Pull request description:

  Closes #931

ACKs for top commit:
  apoelstra:
    ACK 603e75eb77
  sanket1729:
    utACK 603e75eb77.

Tree-SHA512: 8099e80aa2000b3d1284543b6bfab3edd45f8649519bd09b4d73d250bdb6cce5edf67a1e0e0cec61db23c358daca286061641da5ff5c2a8b4b030d1199707c94
2022-04-01 11:38:45 -07:00
sanket1729 7fa8ce0bd0
Merge rust-bitcoin/rust-bitcoin#926: Remove redundant code computing tap hashes
f3ebfd6f8b Remove repeated tap branch hash computing logic (Dr Maxim Orlovsky)
1b28375658 Abstract tap branch hash computing into a dedicated method (Dr Maxim Orlovsky)

Pull request description:

ACKs for top commit:
  apoelstra:
    ACK f3ebfd6f8b

Tree-SHA512: d66d544df324a7d25c8cc9dc48ddedf086ac87eb2ed09a8d7a568cc1488ae44e0807d53ccb7a6e61dbeef0d3d62a1cacf0d69ba7b8de9178ac5c13bae944d08b
2022-04-01 11:35:14 -07:00
Andrew Poelstra 9316c52946
Merge rust-bitcoin/rust-bitcoin#917: Rename SigHash to Sighash
46c34b3fb7 Fix code comments referring to sighash (Tobin Harding)
8f36c3979c Use sighash not sig_hash in identifiers (Tobin Harding)
c3a167b96b Rename SigHash -> Sighash (Tobin Harding)
52b711c084 Rename InvalidSigHashType -> InvalidSighashType (Tobin Harding)
b84f25584e Rename SigHashCache -> SighashCache (Tobin Harding)
e37652578b Rename PsbtSigHashType -> PsbtSighashType (Tobin Harding)
c19ec339ef Rename NonStandardSigHashType -> NonStandardSighashType (Tobin Harding)
130e27349e Rename SigHashTypeParseError -> SighashTypeParseError (Tobin Harding)
6caba2ed24 Rename SchnorrSigHashType -> SchnorrSighashType (Tobin Harding)
5522454583 Rename EcdsaSigHashType -> EcdsaSighashType (Tobin Harding)

Pull request description:

  Our usage of `SigHash` implies that 'sighash' is _two_ words; 'sighash' is a well known word in the Bitcoin ecosystem it should appear in identifiers as `Sighash`.

  Change various types, variants, and code comments to use sighash as a single word.

  - Patches 1-8 are code changes `s/SigHash/Sighash/g`
  - Patch 9 is code changes `s/sig_hash/sighash/g`
  - Patch 11 is docs fixes

  Fixes: #911

  ## Note to reviewers

  I've been particularly pedantic with the patch separation because we are so close to release.

  Done as separate patches to make review easier if review is to be done by reading the diffs. Perhaps at least one person could verify this PR programmatically by doing
  - Reset the last 2 patches (those are easy to do manually)
  - Check out master
  - Do `s/SigHash/Sighash/g` on all source files (bash function below)
  - Use `git diff branchA..branchB` to verify

  The difference between the two branches should only include comment lines (last three patches) and these seven instances of `SigHash:

  ```
  CHANGELOG.md:82:- [Add FromStr/Display implementation for SigHashType](a4a7035a94)
  CHANGELOG.md:93:- [Introduce `SigHashCache` structure](https://github.com/rust-bitcoin/rust-bitcoin/pull/390) to replace `SighashComponents` and support all sighash modes
  CHANGELOG.md:121:    - `SigHash`
  src/blockdata/transaction.rs:1190:            "SigHash_None",
  src/blockdata/transaction.rs:1191:            "SigHash_NONE",
  src/util/sighash.rs:1175:            "SigHash_None",
  src/util/sighash.rs:1176:            "SigHash_NONE",
  ```

  In case its useful, the shell function I used to do these changes is:
  ```bash
  function search-and-replace() {
      if (($# != 2))
      then
          echo "Usage: $0 <this> <that>"
          return
      fi

      local this="$1"
      local that="$2"

      # For all files containing $this, replace $this with $that.
      for file in $(git grep -l "$this")
      do
          perl -pi -e "s/$this/$that/g" "$file"
      done
  }
  ```

ACKs for top commit:
  dr-orlovsky:
    ACK 46c34b3fb7
  apoelstra:
    ACK 46c34b3fb7

Tree-SHA512: fe7e25e9cfb5155e4921de5ac185dbf9f4ca0770846d7892f6968b44fc5431f3f1a183380107449e90f7ea662094c60b118dc0468230384e8f9a8ef98d5ee0a0
2022-04-01 17:30:42 +00:00
Dr. Maxim Orlovsky 3f04c04b3d
Merge rust-bitcoin/rust-bitcoin#920: Push key xonly
f27c4a541d Added push_x_only_key(..) and its test. (mpls)

Pull request description:

  **Issue**

  I can not use [`XOnlyPublicKey`](ae985dd191/src/key.rs (L973)) in my Scripts which prevents me from working with Taproot.

  **Cause**

  The current version of [`script::Builder`](0a2d45de09/src/blockdata/script.rs (L121)) does not accept `XOnlyPublicKey`s.

  **Solution**

  So, I created a function `push_xkey(self, key: &XOnlyPublicKey)` based on the existing [`push_key`](0a2d45de09/src/blockdata/script.rs (L914)) function. I also augmented an [existing test](0a2d45de09/src/blockdata/script.rs (L1108)) in an attempt to reach testing parity with existing code.

  After toying around with `push_xkey`, it seems to work on my end.

ACKs for top commit:
  dr-orlovsky:
    ACK f27c4a541d
  sanket1729:
    utACK f27c4a541d. Thanks a lot for keeping up the iterations with prompt responses

Tree-SHA512: 064958d49edc1d3636a21e428d62c2e9bcd9b13bd226c5821db9e04ce78663a11fcf601c7667b564f88e845207219a052e1c7413f50e5d27c79003e8129825ed
2022-04-01 19:30:55 +03:00
sanket1729 efbe1417fe
Merge rust-bitcoin/rust-bitcoin#923: Taproot docs nits
da731c4825 Add further description to the NodeInfo struct (Tobin Harding)
492ccebd99 Use links for error types (Tobin Harding)
3e05887579 Use 'the' to improve sentence (Tobin Harding)

Pull request description:

  See to nits from review of https://github.com/rust-bitcoin/rust-bitcoin/pull/912

  Three minor patches to the `taproot` module docs.

  CC @dr-orlovsky

ACKs for top commit:
  dr-orlovsky:
    ACK da731c4825
  sanket1729:
    ACK da731c4825

Tree-SHA512: 17a27a19c88f9baa8127023b2ee30fc2259cb0058a92dc9d8ae595e9e02ccb047fefcba7548ff7900fffa7bc6853447183e80660b8756d90d055ab8aa96ae938
2022-04-01 08:55:57 -07:00
Dr Maxim Orlovsky 603e75eb77 Derive Eq for PSBT types 2022-04-01 11:45:32 +02:00
mpls f27c4a541d Added push_x_only_key(..) and its test. 2022-04-01 01:33:00 -05:00
Dr Maxim Orlovsky f3ebfd6f8b
Remove repeated tap branch hash computing logic 2022-03-31 15:21:36 +02:00
Dr Maxim Orlovsky 1b28375658
Abstract tap branch hash computing into a dedicated method 2022-03-31 15:16:39 +02:00
Dr Maxim Orlovsky e3f173e521
Require taproot tree depth argument always to be u8 2022-03-31 15:12:05 +02:00
Tobin Harding da731c4825 Add further description to the NodeInfo struct
Further assist devs in understanding the `NodeInfo` struct by adding
docs about when/why the struct is used.
2022-03-31 10:57:37 +11:00
Tobin Harding 492ccebd99 Use links for error types
We can help the users by linking them to errors when mentioning them in
the docs.
2022-03-31 10:57:05 +11:00
Tobin Harding 3e05887579 Use 'the' to improve sentence
Adding a 'the' makes this sentence a little better.
2022-03-31 10:56:36 +11:00
Tobin Harding 46c34b3fb7 Fix code comments referring to sighash
Recently we added a bunch of additional sighash types, some of the code
comments became stale. Use the non-specific term 'sighash type' instead
of a particular sighash identifier in comments to make the comments more
applicable.
2022-03-31 09:44:22 +11:00
Tobin Harding 8f36c3979c Use sighash not sig_hash in identifiers
Recently we update all types and docs to use `Sighash` instead of
`SigHash` because 'sighash' is a single word. We should apply the same
logic to functions and variable names.

Do not use an underscore in the identifier 'sighash'.
2022-03-31 09:42:52 +11:00
Tobin Harding c3a167b96b Rename SigHash -> Sighash
Our usage of `SigHash` implies that 'sighash' is _two_ words; 'sighash'
is a well known word in the Bitcoin ecosystem it should appear in
identifiers as `Sighash`.

Rename the `SigHash` type to `Sighash`.
2022-03-31 09:42:52 +11:00
Tobin Harding 52b711c084 Rename InvalidSigHashType -> InvalidSighashType
Our usage of `SigHash` implies that 'sighash' is _two_ words; 'sighash'
is a well known word in the Bitcoin ecosystem it should appear in
identifiers as `Sighash`.

Rename the `InvalidSigHashType` variant to `InvalidSighashType`.
2022-03-31 09:42:52 +11:00
Tobin Harding b84f25584e Rename SigHashCache -> SighashCache
Our usage of `SigHash` implies that 'sighash' is _two_ words; 'sighash'
is a well known word in the Bitcoin ecosystem it should appear in
identifiers as `Sighash`.

Rename `SigHashCache` to `SighashCache`.
2022-03-31 09:42:52 +11:00
Tobin Harding e37652578b Rename PsbtSigHashType -> PsbtSighashType
Our usage of `SigHash` implies that 'sighash' is _two_ words; 'sighash'
is a well known word in the Bitcoin ecosystem it should appear in
identifiers as `Sighash`.

Rename `PsbtSigHashType` to `PsbtSighashType`.
2022-03-31 09:42:18 +11:00
Tobin Harding c19ec339ef Rename NonStandardSigHashType -> NonStandardSighashType
Our usage of `SigHash` implies that 'sighash' is _two_ words; 'sighash'
is a well known word in the Bitcoin ecosystem it should appear in
identifiers as `Sighash`.

Rename the `NonStandardSigHashType` type and error variant to
`NonStandardSighashType`.
2022-03-31 09:42:18 +11:00
Tobin Harding 130e27349e Rename SigHashTypeParseError -> SighashTypeParseError
Our usage of `SigHash` implies that 'sighash' is _two_ words; 'sighash'
is a well known word in the Bitcoin ecosystem it should appear in
identifiers as `Sighash`.

Rename `SigHashTypeParseError` to `SighashTypeParseError`.
2022-03-31 09:42:18 +11:00
Tobin Harding 6caba2ed24 Rename SchnorrSigHashType -> SchnorrSighashType
Our usage of `SigHash` implies that 'sighash' is _two_ words; 'sighash'
is a well known word in the Bitcoin ecosystem it should appear in
identifiers as `Sighash`.

Rename `SchnorrSigHashType` to `SchnorrSighashType`.
2022-03-31 09:42:18 +11:00
Tobin Harding 5522454583 Rename EcdsaSigHashType -> EcdsaSighashType
Our usage of `SigHash` implies that 'sighash' is _two_ words; 'sighash'
is a well known word in the Bitcoin ecosystem it should appear in
identifiers as `Sighash`.

Rename `EcdsaSigHashType` to `EcdsaSighashType`.
2022-03-31 09:42:18 +11:00
Dr. Maxim Orlovsky 58a958e3f7
Merge rust-bitcoin/rust-bitcoin#912: Improve docs in taproot module
c25eddd187 Remove unnecessary documentation (Tobin Harding)
8631474f08 Improve docs in taproot module (Tobin Harding)

Pull request description:

  I should have done this PR a month ago, my bad. This one is kind of important IMO because we are going to have so many people looking at this part of the code soon as we release.

  As has been done in other places in the codebase; improve the docs in the `taproot` module by doing:

  - Use full sentences (capital letters + full stops)
  - Use back ticks and links for types where appropriate
  - Fix grammar
  - Fix stale docs
  - Use third person for describing functions
  - Use 100 character line width
  - Use markdown sections (`# Examples`, `# Returns`) where appropriate
  - Separate brief heading from extended description when appropriate
  - Use `///` for all functions/types (both private and public)

  I also did:

  - Build the docs and check all the links
  - Read all the built docs, check for sanity and pretty-ness

  Its all in one patch, I couldn't really tease it apart. I can try a bit harder if it proves too annoying to review.

ACKs for top commit:
  sanket1729:
    ACK c25eddd187
  dr-orlovsky:
    ACK c25eddd187
  apoelstra:
    ACK c25eddd187

Tree-SHA512: 72f35bf8779392060388db985df5abc42a89796eaad1eafd08ea50b635d469fbd07a53ff253cdf27ad4d4baed7d37cec6ea1da1aece3672b9447f87181e218f8
2022-03-30 19:30:14 +03:00
Tobin Harding 8d602b8778 Fix deprecated since version
We deprecated the `bip143::SigHashCache` in

```
commit 53d0e176d3
Author: <elided>
Date:   Fri Jul 16 10:44:18 2021 +0200

    Deprecate bip143::SigHashCache in favor of sighash::SigHashCache

    ...
```

This means these changes are unreleased so the deprecated since version
should be the upcoming 0.28 release.
2022-03-29 10:59:17 +11:00
Tobin Harding c25eddd187 Remove unnecessary documentation
We have some text quoted directly from BIP341, this text is on the net
if folk wish to read it, we don't need it in the source code.
2022-03-29 10:28:29 +11:00
Tobin Harding 8631474f08 Improve docs in taproot module
As has been done in other places in the codebase; improve the docs in
the `taproot` module by doing:

- Use full sentences (capital letters + full stops)
- Use back ticks and links for types where appropriate
- Fix grammar
- Fix stale docs
- Use third person for describing functions
- Use 100 character line width
- Use markdown sections (`# Examples`, `# Returns`) where appropriate
- Separate brief heading from extended description when appropriate
- Use `///` for all functions/types (both private and public)

I also did:

- Build the docs and check all the links
- Read all the built docs, check for sanity and pretty-ness
2022-03-29 10:27:45 +11:00
Dr Maxim Orlovsky 174a99cd06
Implement serde for TweakedKeyPair 2022-03-28 21:29:04 +02:00
Dr Maxim Orlovsky df3297c34e
Implement derives for TweakedKeyPair 2022-03-28 21:28:43 +02:00
Andrew Poelstra b32d40390c
Merge rust-bitcoin/rust-bitcoin#898: Make PsbtSigHashType use the same formatting as other *SigHashTypes
992857ad0a PsbtSighashType unit tests (Dr Maxim Orlovsky)
5be1cdb8c7 PsbtSigHashType Display and FromStr implementation (Dr Maxim Orlovsky)
7cdcdaad6c Support SIGHASH_RESERVED in SchnorrSigHashType::from_u8 (Dr Maxim Orlovsky)

Pull request description:

  The newly introduced `PsbtSigHashType` uses very different serde formatting from previously used `EcdsaSigHashType`; for instance it does not output human-readable sighash. This is especially obvious when printing out PSBT as JSON/YAML object and is a breaking change from the `0.27`. Serde human-readable implementation requires `Display/FromStr`, which were also absent.

ACKs for top commit:
  sanket1729:
    ACK 992857ad0a. This is much better
  apoelstra:
    ACK 992857ad0a

Tree-SHA512: 71a46471f34b5481e4c1273a66846f59d61bfd98fcb65e7823ca216ff0dd419d81ca86d99c7aaf674fcfe2b1c010e899c8e74328f60a1e809015c663c453cc89
2022-03-28 17:34:20 +00:00
Andrew Poelstra 0d5565e131
Merge rust-bitcoin/rust-bitcoin#839: feat: Add Address.is_related_to_*_key()
51fef76129 feat: Add Address.is_related_to_pubkey() (Andrew Ahlers)

Pull request description:

  ## Motivation

  This is addressing the second half of this comment: https://github.com/rust-bitcoin/rust-bitcoin/pull/684#issuecomment-1012136845

  > but would accept a PR (or two PRs) that returns Result<bool, UnsupportedAddress> and a method to check if a PublicKey is associated with an address.

  (The first half was addressed [here](https://github.com/rust-bitcoin/rust-bitcoin/pull/819))

  These changes will help build out and improve message signature verification. We don't necessarily need to add it to this crate but it allows for easy verification with something such as:
  1. recovering a pubkey
  2. checking if that pubkey relates to the given address

  ## Possible Improvements

  - There is likely a better name than `is_related_to_secp256k1_key()`
  - This could drop the `secp256k1` part of the name and take in a Pubkey enum that also supports Schnorr pubkeys and then this could be used for taproot addresses as well. This felt like a much larger change that will likely get turned down. Verifying taproot is simple enough and if absolutely desired, similar functions can be added for schnorr keys (tweaked and untweaked)

ACKs for top commit:
  Kixunil:
    ACK 51fef76129 for merging after TR
  apoelstra:
    ACK 51fef76129

Tree-SHA512: c9ab8c0f101fb4c647713e7f500656617025d8741676e8eb8a3132009dde9937d50cf9ac3d8055feb14452324a292397e46639cbaca71cac77af4b06dc42d09d
2022-03-28 17:29:48 +00:00
Dr Maxim Orlovsky 8dabe3ed64
Taproot Huffman tree builder u64->u32 fixes 2022-03-28 17:15:28 +02:00
Andrew Poelstra 7f53c2cdc1
Merge rust-bitcoin/rust-bitcoin#909: Make TaprootBuilder able to generate Huffman Tree
ec17ec356d Move with_huffman_tree logic to TaprootBuilder (Jeremy Rubin)

Pull request description:

  .

ACKs for top commit:
  apoelstra:
    ACK ec17ec356d
  dr-orlovsky:
    utACK ec17ec356d

Tree-SHA512: 67a013124267f64bfae0b2007418ad59a42ae64d8b95e23c1d86cc7d96b0dd3b48deb255ce7bb839ef9a4d4f2e3a42d691d2d2430eb7791e01f992635773cc96
2022-03-28 15:08:21 +00:00
Dr Maxim Orlovsky 992857ad0a
PsbtSighashType unit tests 2022-03-28 17:03:44 +02:00
Dr Maxim Orlovsky 5be1cdb8c7
PsbtSigHashType Display and FromStr implementation 2022-03-28 17:03:34 +02:00
Dr Maxim Orlovsky 7cdcdaad6c
Support SIGHASH_RESERVED in SchnorrSigHashType::from_u8 2022-03-28 16:53:31 +02:00
Andrew Poelstra 10949b7177
Merge rust-bitcoin/rust-bitcoin#910: Make NodeInfo API public
208eb65f1b Make NodeInfo API public (sanket1729)

Pull request description:

  Reported by @shesek. Users might find it convenient to manually construct the tree using `NodeInfo` API

  ```rust
  let leaf1 = NodeInfo::from_leaf_with_ver();
  let leaf2 = NodeInfo::from_leaf_with_ver();

  let root = NodeInfo::combine(leaf1, leaf2);
  let spend_info = TaprootSpendInfo::from_node_info(&secp, internal_key, root);
  ```

ACKs for top commit:
  dr-orlovsky:
    ACK 208eb65f1b
  apoelstra:
    ACK 208eb65f1b

Tree-SHA512: b5a6b26e0d4a637f7ad6e987976b31b00d3567feca85f1a0bf63aa03603aded0ddae6578b1cabc1056870a596b8cb1a83e4ef3f45802e03da80c3d58d9bab1f1
2022-03-28 14:02:26 +00:00
Andrew Poelstra 388897bf93
Merge rust-bitcoin/rust-bitcoin#901: TapTree iterator
e27f8ff594 TapTree iterator implementation (Dr Maxim Orlovsky)

Pull request description:

  Implemented after @sanket1729 suggestion in https://github.com/rust-bitcoin/rust-bitcoin/issues/895#issuecomment-1074366108

  Iterates all scripts present in TapTree in DFS order returning `(depth, script)` pairs.

  I propose to have it as an RC fix since this functionality is really lacking and may be required for many wallets working with Taproot PSBT even outside of the scope where I originally needed it (OP_RETURN tweaks for TapTree described in #895)

ACKs for top commit:
  sanket1729:
    utACK e27f8ff594.
  apoelstra:
    ACK e27f8ff594

Tree-SHA512: b398e468a10534561297f22dba47e340391069734a41999edd85d726890752035053690a22014402879ea40b948160f00310f78771443d382c0bbaf0201dfbe5
2022-03-28 13:45:34 +00:00
Dr. Maxim Orlovsky c3d30d51a7
Remove deprecated method use for sighash conversion
Post-merge #796 follow-up. Feel free to add other changes/nits which hadn't get into #796.
2022-03-28 09:54:13 +02:00
sanket1729 208eb65f1b Make NodeInfo API public
This allows users to create TaprootSpendInfo using NodeInfo. This
offers an alternative to TaprootBuilder.
2022-03-27 17:34:05 -07:00
Tobin Harding 8e2422f92b Add unit test for deserialize non-standard sighash
It is possible, although not immediately obvious, that it is possible to
create a `PsbtSigHashType` with a non-standard value.

Add a unit test to show this and also catch any regressions if we
accidental change this logic.
2022-03-28 10:43:37 +11:00
Tobin Harding e05776f176 Improve PsbtSigHashType conversion methods
Improve the `PsbtSigHashType` conversion methods by doing:

- Re-name `inner` -> `to_u32` as per Rust convention
- Add `from_u32` method

Note, we explicitly do _not_ use suffix 'consensus' because these
conversion methods make no guarantees about the validity of the
underlying `u32`.
2022-03-28 10:43:37 +11:00
Tobin Harding ac462897b1 Remove hungarian-ish notation
The functions `from_u32_standard` and `from_u32_consensus` smell a bit
like hungarian notation. We can look at the method definition to see
that the methods accept `u32` arguments without mentioning that in the
method names.

Remove `_u32_` from the method names. This brings the `from_*` methods
in line  with the `to_standard` method also.
2022-03-28 10:43:37 +11:00
Tobin Harding 564682627c Remove deprecated conversion method
`EcdsaSigHashType::from_u32` was deprecated in v0.26, since we are
working on the v0.28 release we can drop this method.
2022-03-28 10:43:06 +11:00
Tobin Harding d1753d7ff1 Rename as_u32 -> to_u32
Rust naming conventions stipulate that conversion methods from owned ->
owned for `Copy` types use the naming convention `to_`.

This change makes the function name objectively better, however it makes
no claims of being the 'best' name. We have had much discussion on using
`to_standard` vs `to_u32` but are unable to reach consensus.
2022-03-28 10:43:06 +11:00
Tobin Harding 2bd71c3748 Remove From<EcdsaSigHashType> for u32
We have conversion functions that include suffixes `_consensus`
and `_standard` to make it explicit what guarantees are provided by the
returned `u32` value. The `From` implementation reduces the clarity of
the API.
2022-03-28 10:43:06 +11:00
Dr. Maxim Orlovsky b7adaf69f3
Merge rust-bitcoin/rust-bitcoin#907: Add Serialize/Deserialize for TaprootSpendInfo
2b942cf506 Add Serialize/Deserialize for TaprootSpendInfo (Jeremy Rubin)

Pull request description:

  I think this is missing -- unless there is a reason not to have it?

ACKs for top commit:
  apoelstra:
    ACK 2b942cf506
  dr-orlovsky:
    ACK 2b942cf506

Tree-SHA512: d1467d8515c85a5057037b1e5bf53c1930275fbe7e4fcbc726079a47febd75d6bbce8e2d99ed4f9d8afccf6fc3782e43763a2258c4c2a934c2453920fe587e4b
2022-03-27 14:19:15 +03:00
Dr. Maxim Orlovsky 8884a61283
Merge rust-bitcoin/rust-bitcoin#908: Add serde to TweakedPublicKey
204f477a34 Add serde to TweakedPublicKey (Jeremy Rubin)

Pull request description:

ACKs for top commit:
  dr-orlovsky:
    ACK 204f477a34
  apoelstra:
    ACK 204f477a34
  sanket1729:
    utACK 204f477a34

Tree-SHA512: 61b0140549dfa641edee7e45dbb9f9c816d7319eb83d00ad9b0e7e499f3273de98b65c9905e7d50d4b6ecfd084b8848c4e612cb441124cc469142304ebff18d9
2022-03-27 13:53:30 +03:00
Andrew Poelstra 734b1deb70
Merge rust-bitcoin/rust-bitcoin#897: Check for SIGHASH_SINGLE bug in writer fn
83dda74ecb Check for SIGHASH_SINGLE bug in writer fn (Tobin Harding)

Pull request description:

  Recently we moved the logic for checking for the SIGHASH_SINGLE bug to
  the `signature_hash()` function. Although this left users of the
  `encode_signing_data_to()` function without correct handling of the bug
  there is not much else we can do but alert users to this behaviour.

  Add documentation to highlight the behaviour of `encdoe_signing_data_to`
  in regards to the sighash single bug. Requires updating docs for
  `signature_hash` also.

  Please note, uses non-conventional markdown header `# Warning`.

  Closes: #817

ACKs for top commit:
  sanket1729:
    ACK 83dda74ecb. This is much cleaner
  dr-orlovsky:
    ACK 83dda74ecb
  apoelstra:
    ACK 83dda74ecb

Tree-SHA512: 1263b06ddfbb05a293c80e7dbf6f87eac5922c501e7db1c1d26d41d3ea0172c6b7a44afc0b1843b06e78985d3ecf70a3a3feb2515d535a7413685aed0a338c64
2022-03-26 00:56:37 +00:00
Jeremy Rubin ec17ec356d Move with_huffman_tree logic to TaprootBuilder 2022-03-24 14:57:30 -07:00
Jeremy Rubin 2b942cf506 Add Serialize/Deserialize for TaprootSpendInfo 2022-03-24 14:40:27 -07:00
Jeremy Rubin 204f477a34 Add serde to TweakedPublicKey 2022-03-24 14:39:55 -07:00
Dr. Maxim Orlovsky d263c0c31e
Merge rust-bitcoin/rust-bitcoin#881: Remove feature gated enum variants
6ad2902814 Remove feature gated enum variants (Tobin Harding)

Pull request description:

  This is the updated version of #874 (which I closed, force pushed, and then was unable to re-open - my bad).

  Feature gating enum variants makes code that uses the library brittle while we do not have `non_exhaustive`, we should avoid doing so. Instead we can add a dummy type that is available when the feature is not turned on. Doing so enables the compiler to enforce that we do not create the error type that is feature gated when the feature is not enabled.

  Remove the feature gating around `bitcoinconsensus` error enum variants.

  Closes: #645

ACKs for top commit:
  sanket1729:
    tACK 6ad2902814. This is an improvment.
  dr-orlovsky:
    ACK 6ad2902814

Tree-SHA512: 07d8c6b500d2d5b92e367b89e296b86bec046bab4fe9f624eb087d52ea24a900d7f7a41a98065949c67b307a1f374a7f4cf1b77cb93b6cf19e3d779c27fd7f1d
2022-03-24 14:49:38 +02:00
Dr. Maxim Orlovsky 86c6ab7529
Merge rust-bitcoin/rust-bitcoin#903: Improve `SchnorrSigHashType`
35b682d495 Implement Display/FromStr for SchnorrSigHashType (Tobin Harding)
46c4164d67 Improve SigHashTypeParseError field (Tobin Harding)
c009210d4c Use full path for String in macro (Tobin Harding)

Pull request description:

  Implement Display/FromStr for SchnorrSigHashType

  We currently implement `Display` and `FromStr` on `EcdsaSigHashType` and use them in the `serde_string_impl` macro to implement ser/de.

  Mirror this logic in `SchnorrSigHashType`.

  Patch 1 and 2 are preparatory patches for patch 3.

  ## Notes to reviewers

  This PR has some conflicts with https://github.com/rust-bitcoin/rust-bitcoin/pull/898 but is pushing in the same direction, I'm happy to let 898 go in first and rebase on top.

ACKs for top commit:
  sanket1729:
    ACK 35b682d495. Thanks, much easier to review now that the diff is small
  dr-orlovsky:
    ACK 35b682d495

Tree-SHA512: 481f192a3064ff39acf8904737dfb25b54ef128a37e0ca765ebb39138edac772d4f01ed10aa98ff185a8ed5668d64fa5d5957206b920ffe87950cafcf5a3b516
2022-03-24 08:02:27 +02:00
sanket1729 ea80e6568a
Merge rust-bitcoin/rust-bitcoin#805: Remove impl_index_newtype macro
63e36fe6b4 Remove impl_index_newtype macro (Tobin Harding)

Pull request description:

  This macro is no longer needed since we bumped MSRV to 1.29.

  ~We can implement `SliceIndex` to get the `Index` implementations.~
  We can implement `core::ops::Index` directly since all the inner types implement `Index` already.

  Original ~Idea shamelessly stolen from @elichai [in this comment](https://github.com/rust-bitcoin/rust-bitcoin/issues/352#issuecomment-560331856).~

  New idea proposed by @Kixunil during review below. Thanks.

ACKs for top commit:
  apoelstra:
    ACK 63e36fe6b4
  dr-orlovsky:
    utACK 63e36fe6b4
  sanket1729:
    ACK 63e36fe6b4

Tree-SHA512: f7b4555c7fd9a2d458dcd53ec8caece0d12f3af77a10e850f35201bd7a580ba8fd7cb1d47a7f78ba6582e777dffa13416916ecacac6e0e874bdbb1c866132dc2
2022-03-23 19:01:56 -07:00
Tobin Harding 35b682d495 Implement Display/FromStr for SchnorrSigHashType
We currently implement `Display` and `FromStr` on `EcdsaSigHashType` and
use them in the `serde_string_impl` macro to implement ser/de.

Mirror this logic in `SchnorrSigHashType`.
2022-03-24 12:47:18 +11:00
Tobin Harding 46c4164d67 Improve SigHashTypeParseError field
In preparation for constructing an error outside of this module improve
the `SigHashTypeParseError` by doing:

- Make the field public
- Rename the field to `unrecognized` to better describe its usage
2022-03-24 12:28:59 +11:00
Tobin Harding 83dda74ecb Check for SIGHASH_SINGLE bug in writer fn
Recently we moved the logic for checking for the SIGHASH_SINGLE bug to
the `signature_hash()` function. Although this left users of the
`encode_signing_data_to()` function without correct handling of the bug
there is not much else we can do but alert users to this behaviour.

Add documentation to highlight the behaviour of `encdoe_signing_data_to`
in regards to the sighash single bug. Requires updating docs for
`signature_hash` also.

Please note, uses non-conventional markdown header `# Warning`.
2022-03-24 11:38:58 +11:00
Tobin Harding c009210d4c Use full path for String in macro
As is done in the rest of the `internal_macros` module use the fully
qualified path for the `String` type.

Done in preparation for using `serde_string_impl` in the `sighash`
module.
2022-03-24 11:01:15 +11:00
Dr Maxim Orlovsky e27f8ff594
TapTree iterator implementation 2022-03-24 00:03:54 +01:00
Steven Roose 31571cafbd
util::amount: Make from_sat constructor constant 2022-03-23 15:15:08 +00:00
Tobin Harding 6ad2902814 Remove feature gated enum variants
Feature gating enum variants makes code that uses the library brittle
while we do not have `non_exhaustive`, we should avoid doing so. Instead
we can add a dummy type that is available when the feature is not turned
on. Doing so enables the compiler to enforce that we do not create the
error type that is feature gated when the feature is not enabled.

Remove the feature gating around `bitcoinconsensus` error enum variants.

Closes: #645
2022-03-23 14:58:45 +11:00
Andrew Poelstra 1295008dc6
Merge rust-bitcoin/rust-bitcoin#861: Remove get_ prefix
3bde1a205c Remove get_ prefix (Tobin Harding)

Pull request description:

  This one might be a viewed as code churn or unnecessarily modifying the API, feel free to NACK :)

  We have a bunch of methods that use the prefix `get_`, they are not exactly getters because they do more than just access a struct fields so Rust convention relating to getters does not apply, however, the `get_` prefix does not add to the descriptiveness of name hence the shorter form can be used with no loss of clarity.

  Improve docs and deprecate any methods changed that are pubic.

ACKs for top commit:
  dr-orlovsky:
    ACK 3bde1a205c
  apoelstra:
    ACK 3bde1a205c
  sanket1729:
    ACK 3bde1a205c

Tree-SHA512: d9e618ba7fec81ad157c2c806d1db273f899d63707c78254c133b619293f9f0c9a4f3a3e091e9aad399479ff80d5d052c424501164374c21bb90fb9783a4824e
2022-03-21 21:34:54 +00:00
Andrew Poelstra 1cde71ea44
Merge rust-bitcoin/rust-bitcoin#880: Use conventional spacing for default type parameters
1629348c24 Use conventional spacing for default type parameters (Tobin Harding)

Pull request description:

  The exact code formatting we use is not as important as uniformity. Since we do not use tooling to control the formatting we have to be vigilant ourselves. Recently I (Tobin) changed the way default type parameters were formatted (arbitrarily but uniformly). Turns out I picked the wrong way, there is already a convention as shown in the rust documentation online (e.g. [1]).

  Use 'conventional' spacing for default type parameters. Make the changeacross the whole repository, found using

      git grep '\<.* = .*\>'

  [1] - https://doc.rust-lang.org/book/ch19-03-advanced-traits.html

  **Note**

  I also audited our whole stack to make sure I had not botched this anywhere else. Apologies for the code churn.

ACKs for top commit:
  dr-orlovsky:
    utACK 1629348c24
  apoelstra:
    ACK 1629348c24

Tree-SHA512: 61c8a22acc557d8b99f7b591cf8f91b94778a954ac1c9d6cf04a2bbd10953c241e8298e71928aad3b065c98117b816b913226d973fdaa9c3a1aea8daf8bcbe72
2022-03-21 21:09:46 +00:00
Dr. Maxim Orlovsky 443a38567b
Merge rust-bitcoin/rust-bitcoin#877: Improve ClassifyContext rustdocs
51a51cd67d Improve ClassifyContext rustdocs (Tobin Harding)

Pull request description:

  Improve the rustdocs on the `ClassifyContext` enum by doing:

  - Use link for `OP_RESERVED`
  - Use term `OP_SUCCESSx` is done in BIP342 (no code link, does not exist in code).
  - Use enum::variant form for both variant mentions
  - Direct readers to BIP342 for full list of opcode re-names

ACKs for top commit:
  sanket1729:
    ACK 51a51cd67d
  apoelstra:
    ACK 51a51cd67d
  dr-orlovsky:
    ACK 51a51cd67d

Tree-SHA512: 1a9067246ef84eae39b0adef64190b9212dacb55a420909ee38c582ef1960fceb572f82d3eeff518b58fc2cceffe71b3da4e78da54cd4cb6e05a0e48a3a9d03c
2022-03-20 20:40:32 +02:00
Tobin Harding 3bde1a205c Remove get_ prefix
We have a bunch of methods that use the prefix `get_`, they are not
exactly getters because they do more than just access a struct fields so
Rust convention relating to getters does not apply, however, the `get_`
prefix does not add to the descriptiveness of name hence the shorter
form can be used with no loss of clarity.

Improve docs and deprecate any methods changed that are pubic.
2022-03-19 08:28:30 +11:00
Dr. Maxim Orlovsky ebf9162835
Merge rust-bitcoin/rust-bitcoin#860: Fix signature hash returned for sighash single bug
d1abfd9c30 Add unit test for sighash single bug (Tobin Harding)
82f29b4267 Use 1 signature hash for invalid SIGHASH_SINGLE (Tobin Harding)
3831816a73 Move test helper function (Tobin Harding)
3e21295b88 Remove unnecessary whitespace character (Tobin Harding)

Pull request description:

  Fix up the logic that handles correctly returning the special array 1,0,0,...,0 for signature hash when the sighash single bug is exploitable i.e., when signing a transaction with SIGHASH_SINGLE for an input index that does not have a corresponding transaction output of the same index.

  - Patch 1 and 2: Clean up
  - Patch 3: Implements the fix
  - Patch 4: Adds a passing test that fails if moved to before patch 3

  Resolves: #817

ACKs for top commit:
  apoelstra:
    ACK d1abfd9c30
  dr-orlovsky:
    ACK d1abfd9c30

Tree-SHA512: f2d09e929d2f91348ae0b0758b3d4be6c6ce0cb38c4988e0bebb29f5918ca8491b9e7b31fe745f7c20d9348612fe2166f0a12b782f256aad5f6b6c027c2218b7
2022-03-18 13:00:09 +02:00
Tobin Harding 1629348c24 Use conventional spacing for default type parameters
The exact code formatting we use is not as important as uniformity.
Since we do not use tooling to control the formatting we have to be
vigilant ourselves. Recently I (Tobin) changed the way default type
parameters were formatted (arbitrarily but uniformly). Turns out I
picked the wrong way, there is already a convention as shown in the rust
documentation online (e.g. [1]).

Use 'conventional' spacing for default type parameters. Make the change
across the whole repository, found using

    git grep '\<.* = .*\>'

[1] - https://doc.rust-lang.org/book/ch19-03-advanced-traits.html
2022-03-18 10:40:51 +11:00
Tobin Harding 63e36fe6b4 Remove impl_index_newtype macro
This macro is no longer needed since we bumped MSRV to 1.29.

We can implement `core::ops::Index` directly since all the inner types
implement `Index` already.
2022-03-17 08:12:09 +11:00
Tobin Harding 51a51cd67d Improve ClassifyContext rustdocs
Improve the rustdocs on the `ClassifyContext` enum by doing:

- Use link for `OP_RESERVED`
- Use term `OP_SUCCESSx` is done in BIP342 (no code link, does not exist
  in code).
- Use enum::variant form for both variant mentions
- Direct readers to BIP342 for full list of opcode re-names
2022-03-15 14:39:24 +11:00
Martin Habovstiak 4f1200d629 Added `amount::Display` - configurable formatting
This significatnly refactors the amount formatting code to make
formatting more configurable. The main addition is the
`amount::Display` type which is a builder that can configure
denomination or other things (possibly more in the future).

Further, this makes all representations of numbers minimal by default,
so should be documented as a possibly-breaking change.

Because of the effort to support all other `fmt::Formatter` options this
required practically complete rewrite of `fmt_satoshi_in`. As a
byproduct I took the opportunity of removing one allocation from there.

Closes #709
2022-03-14 19:03:03 +01:00
Tobin Harding a77907d59c Remove unnecessary explicit type annotation
The compiler can infer this type, no need for an explicit type
annotation.
2022-03-14 13:52:25 +11:00
Tobin Harding 71cf00a314 Use less vertical lines
In this library we specifically do not use rustfmt and tend to favour
terse statements that do not use extra lines unnecessarily. In order to
help new devs understand the style modify code that seems to use an
unnecessary number of lines.

None of these changes should reduce the readability of the code.
2022-03-14 13:52:13 +11:00
Tobin Harding a5c06e0a96 Refactor vector initialisation
Vector initialisation uses neither "Block" nor "Visual" stlye, this is
irregular for no added benefit.

Elect to use "Block" style (as defined by `rustfmt`).
2022-03-14 13:52:13 +11:00
Tobin Harding aabf2d1681 Use brace not parenth fo macro arm
Macro match arms can use any parenthesis-like character (it seems),
however since we are delimiting a block of code elect to use braces.
2022-03-14 13:52:13 +11:00
Tobin Harding b021415a88 Use block stlye function call
This function uses neither "Block" nor "Visual" style (as defined by
`rustfmt`). This is unusual, code that is regular is less jarring to
read. We tent to use "Block" style for functions so elect to do that
here.
2022-03-14 13:52:13 +11:00
Tobin Harding d6462bae7b Refactor usage of +
As we do for logical operators; put the `+` operator at the start of the
line to make it more obvious and assist devs reading the code.
2022-03-14 13:52:13 +11:00
Tobin Harding 702e8bf82d Refactor consensus_encode
The implementations of `consensus_encode` use an unnecessary number of
lines. Favour more terse code with no loss of clarity.
2022-03-14 13:52:13 +11:00
Tobin Harding a8ed95ea07 Refactor where statements
Our usage of `where` statements is not uniform, nor is it inline with
the typical layout suggested by `rustfmt`.

Make an effort to be more uniform with usage of `where` statements.
However, explicitly do _not_ do every usage since sometimes our usage
favours terseness (all on a single line).
2022-03-14 13:52:13 +11:00
Tobin Harding 6d84998168 Improve braces usage
Add a pair of braces to improve readability.
2022-03-14 13:52:13 +11:00
Tobin Harding 39ec59620d Fix unusual indentation
We have a few instances of strange indentation:

- Incorrect number of characters
- Usage of neither "Block" style or "View" style (elect to use "Block")
2022-03-14 13:52:13 +11:00
Tobin Harding b9b6e7e1c6 Remove unneeded braces
Use statement contains unneeded braces, remove them.
2022-03-14 13:52:13 +11:00
Tobin Harding 5d68ad85ed Remove unneeded return statement
The last statement of a function does not need an explicit `return`
statement.
2022-03-14 13:52:13 +11:00
Tobin Harding bf4f5638e0 Refactor whitespace
Do various whitespace refactorings, of note:

- Use space around equals e.g., 'since = "blah"'
- Put return/break/continue on separate line

Whitespace only, no logic changes.
2022-03-14 13:51:50 +11:00
Tobin Harding 1c502399f1 Remove trailing whitespace
Remove trailing whitespace from all rust source files.

Done with:

find . -name *.rs | xargs perl -pli -e "s/\s*$//"
2022-03-14 13:40:44 +11:00
Tobin Harding d1abfd9c30 Add unit test for sighash single bug
When signing a transaction will result in the sighash single bug being
exploitable we should return the 'one array' (equivalent to 1 as a
uint256) as the signature hash.

Add a unit test to verify we return uint256 1 value when use of
SIGHASH_SINGLE is invalid.
2022-03-14 12:10:16 +11:00
Tobin Harding 82f29b4267 Use 1 signature hash for invalid SIGHASH_SINGLE
When signing a transaction will result in the sighash single bug being
exploitable we should return the 1 array (equivalent to 1 as a uint256)
as the signature hash.

Currently we are using the correct array value but are re-hashing it,
instead we should directly return it.
2022-03-14 12:09:39 +11:00
Tobin Harding 3831816a73 Move test helper function
Move helper function to above the test that uses it.

Refactor only, no logic changes.
2022-03-14 12:06:54 +11:00
Tobin Harding 3e21295b88 Remove unnecessary whitespace character
Typically we do not put a whitespace character before a `:` when using
explicit types.
2022-03-14 12:06:54 +11:00
Dr Maxim Orlovsky 7554d76dfe
Make Script::witness_version public 2022-03-13 14:40:03 +01:00
Andrew Poelstra 93daed95bd
Merge rust-bitcoin/rust-bitcoin#871: Delete contract hash module
7f33fe6a9b Delete contract hash module (Tobin Harding)

Pull request description:

  This module has been deprecated in commit 1ffdce9 in August 2020, it is safe to delete it now.

  Fixes: #322

ACKs for top commit:
  apoelstra:
    ACK 7f33fe6a9b
  Kixunil:
    ACK 7f33fe6a9b
  dr-orlovsky:
    ACK 7f33fe6a9b

Tree-SHA512: f218c8b0c09b14cd885cd7cf03c0a4623e5ead785decbc62a2f9610d438d5ea3efd2e2b47172a7608e33714996efa121707583d4257fa683dbfc9717988ceda6
2022-03-12 12:27:31 +00:00
Dr. Maxim Orlovsky 60d941621d
Merge rust-bitcoin/rust-bitcoin#825: test: Add a test for incorrect message signature in `is_signed_by_address`
e391ce9939 test: Add a test for incorrect message signature (Andrew Ahlers)

Pull request description:

  In response to this comment: https://github.com/rust-bitcoin/rust-bitcoin/pull/819#discussion_r801477961

  This should be straightforward. Let me know if there are any style issues. I tried to keep things similar to the existing test while cutting out any extra cruft to keep things small.

ACKs for top commit:
  apoelstra:
    ACK e391ce9939
  Kixunil:
    ACK e391ce9939
  dr-orlovsky:
    ACK e391ce9939

Tree-SHA512: 47296a7e0b2f45d5e50f507727ae4360686730a386f37dedfd1360b8cdf4b9dd3ce3bb5d05ea630177379ce4109059b6924fa362396b984ebab0ed1754318627
2022-03-12 13:17:45 +02:00
Dr. Maxim Orlovsky 377f4ad89f
Merge rust-bitcoin/rust-bitcoin#862: Flatten the policy module
ac105903cd Flatten the policy module (Tobin Harding)

Pull request description:

  The policy module contains a single `mod.rs` file, this is unnecessary, we can simply use `policy.rs` and flatten the module.

ACKs for top commit:
  apoelstra:
    utACK ac105903cd
  dr-orlovsky:
    ACK ac105903cd

Tree-SHA512: b0a9d2a68697a61fd85c1f4471c8df5fdcd7aa7052c33b4db385c311db96d3a6bcc80f17414ecae7f37f15fb0c8dc9f7ceaaf89cc1375f77fb2a5c489b948894
2022-03-12 13:08:30 +02:00
Dr. Maxim Orlovsky 77606117cc
Merge rust-bitcoin/rust-bitcoin#863: Trivial: Order impl_hashencode lines
ee3b8c267d Order impl_hashencode lines (Tobin Harding)

Pull request description:

  Put the calls to `impl_hashencode` in the same order, and with the same
  whitespace, as the calls to `hash_newtype`. This makes groking the file
  easier because its quick to glance down the types and see which ones
  implement hashencode (consensus_encode/decode) and which ones do not.

ACKs for top commit:
  apoelstra:
    ACK ee3b8c267d
  dr-orlovsky:
    ACK ee3b8c267d

Tree-SHA512: 77f43fb65bdf0020c713b94bd8413c320e3acd6a39f28c1a89d8f0d29893f4559993fa864c490332ead262f03f05519a483d883af6b031889b5634fcf1e6cfe7
2022-03-12 12:54:24 +02:00
Dr. Maxim Orlovsky 40e6f131ff
Merge rust-bitcoin/rust-bitcoin#867: Cleanup opcodes module
f4886afa66 Add full stops to docs (Tobin Harding)
f01f047b21 Remove unnecessary newlines (Tobin Harding)
8a1cc2ca77 Improve docs on ClassifyContext (Tobin Harding)

Pull request description:

  Do some clean ups to the `blockdata::opcodes` module. Patch 3 is big but it should be quick to review because I made all the boring 'add full stops' changes in a single commit.

ACKs for top commit:
  Kixunil:
    ACK f4886afa66
  apoelstra:
    ACK f4886afa66
  dr-orlovsky:
    ACK f4886afa66

Tree-SHA512: b30f36bd06a028b6bbc24a64849c0788a9223760907bdcb3765af1742a228f630cc7666ed66fa2afd8fb6c96e3cf416e9bd9d2a3b6c72c6e47a16399a856fca1
2022-03-12 12:50:40 +02:00
Dr. Maxim Orlovsky 7e755d3ddd
Merge rust-bitcoin/rust-bitcoin#864: Cleanup block module
146d5e83d1 Improve docs for blockdata::block (Tobin Harding)
f03092c380 Fix erroneous function rustdoc (Tobin Harding)
5464848f45 Refactor check_witness_commitment (Tobin Harding)

Pull request description:

  Do some clean ups to the `blockdata::block` module.

  - Patch 1: Change predicate names (API breaking, could be seen as unnecessarily changing the API), can remove if NACK'd
  - Patch 2: Refactor to assist code clarity
  - Patch 3 and 4: are docs improvements, shouldn't be too controversial

ACKs for top commit:
  apoelstra:
    ACK 146d5e83d1
  dr-orlovsky:
    ACK 146d5e83d1

Tree-SHA512: 65cc414857c4569a389638b53eb99ed629bf67ae1d8ebdc9023e5974bb26902d4de41ec311bef3b5c895229d7d0df78d469a84c1e94fc0b7be7435338f0d510a
2022-03-12 12:41:08 +02:00
Dr. Maxim Orlovsky 501cf63f0d
Merge rust-bitcoin/rust-bitcoin#865: Cleanup transaction docs
e503f14331 Improve docs: blockdata::transaction (Tobin Harding)
f02b3a8472 Add code comment for emtpy input (Tobin Harding)
6a0ec1ac47 Remove redundant _eq (Tobin Harding)
3bcc146a44 Improve docs: encode_signing_data_to/signature_hash (Tobin Harding)

Pull request description:

  Do some cleanups to the docs in `blockdata::transaction`. Patch 1 needs the most careful review please. The rest should not be too controversial.

ACKs for top commit:
  apoelstra:
    ACK e503f14331
  dr-orlovsky:
    ACK e503f14331

Tree-SHA512: 3953226e1b7f0db0371b1902888407a48531688bf8ed08539a0090f369b491b130d70b2fae859878ef178a397cefe0ee2a15f3358afc990a2776194cc2b3882b
2022-03-12 12:12:11 +02:00
Dr. Maxim Orlovsky 64164db4b4
Merge rust-bitcoin/rust-bitcoin#869: Improve docs: script module
4dcbef6ddd Improve docs: script module (Tobin Harding)

Pull request description:

  Improve the docs in the `blockdata::script` module by doing:

  - Use full sentences (use capitals and full stops)
  - Improve grammar/wording if necessary
  - Remove incorrect/unneeded comments
  - Fix layout of rustdoc i.e., use brief and description sections
  - Use 100 line character width if it makes the comment look better
  - Use third person instead of imperative tense

  ## Note to reviewers

  Sorry to be a bore and request review on all these docs fixes, this one is all in a single patch which makes it a bit harder to review. It is very similar in content to all the others that are open right now so I'm going to be a bit rude and leave it like this. Please say if this is even slightly putting too much demand on you review time.

ACKs for top commit:
  apoelstra:
    ACK 4dcbef6ddd
  dr-orlovsky:
    ACK 4dcbef6ddd

Tree-SHA512: 49fa1d88c4b97decbc563747ba166fe95698da6a634801ccf5f99fd67a4a907067dbf0a4d64e7773d5d5b04aef404167b6cc911382363247d15a61cef5d8965c
2022-03-12 12:08:32 +02:00
Dr. Maxim Orlovsky 7f0d1720d6
Merge rust-bitcoin/rust-bitcoin#875: Update secp256k1 dependency
d68531d815 Update secp256k1 dependency (Tobin Harding)

Pull request description:

  Update our `rust-secp256k1` dependency to the latest released version.

  Requires doing:

  - Add a new variant to `Error` for the case where parity of the internal key is an invalid value (not 0 or 1).
  - Use non-deprecated const

  Please check the error change carefully, this error does relate _only_ to the parity of an internal key, right?

ACKs for top commit:
  apoelstra:
    ACK d68531d815
  dr-orlovsky:
    ACK d68531d815

Tree-SHA512: 2552b07c0ccc065ced412caadaa0e9d8d77b5f2ce3698b7f53367a9f183557172526c154594c1c706e229da1bab67d11d88255cfd1fe3aac3e16888fe2948aae
2022-03-12 08:00:47 +02:00
Tobin Harding d68531d815
Update secp256k1 dependency
Update our `rust-secp256k1` dependency to the latest version.

Requires doing:

- Add a new variant to `Error` for the case where parity of the internal
  key is an invalid value (not 0 or 1).
- Use non-deprecated const
2022-03-12 08:12:42 +11:00
Tobin Harding 7f33fe6a9b Delete contract hash module
This module has been deprecated in commit 1ffdce9 in August 2020, it is
safe to delete it now.

Fixes: #322
2022-03-10 08:58:25 +11:00
Tobin Harding 4dcbef6ddd Improve docs: script module
Improve the docs in the `blockdata::script` module by doing:

- Use full sentences (use capitals and full stops)
- Improve grammar/wording if necessary
- Remove incorrect/unneeded comments
- Fix layout of rustdoc i.e., use brief and description sections
- Use 100 line character width if it makes the comment look better
- Use third person instead of imperative tense
2022-03-10 07:45:38 +11:00
Tobin Harding e503f14331 Improve docs: blockdata::transaction
Improve the rustdocs for the `blockdata::transaction` module:

- Use full sentences (capitalisation and full stop)
- Use third person tense instead of imperative
- Improve wording/grammar
- Use backticks in links
- Use 100 character column width if it improves readability

Nothing too controversial here :)
2022-03-10 07:20:40 +11:00
Tobin Harding f02b3a8472 Add code comment for emtpy input
The line of code `let mut have_witness = self.input.is_empty();` is
puzzling if one does not know _why_ we serialize in BIP141 style when
there are no inputs.

Add a code comment to save devs spending time trying to work out _why_
this is correct.
2022-03-10 07:14:06 +11:00
Tobin Harding 6a0ec1ac47 Remove redundant _eq
`assert!` already checks a boolean, it is redundant to use `assert_eq!`
and pass in `true`.

Remove redundant usage of `assert_eq!(foo, true)`.
2022-03-10 07:13:55 +11:00
Tobin Harding 3bcc146a44 Improve docs: encode_signing_data_to/signature_hash
The two methods `encode_signing_data_to` and `signature_hash` use the
same docs (one is a public helper for the other). The docs have gotten a
bit stale (refer to deprecated types).

Instead of duplicating all the text, add a statement pointing readers
from the docs of `signature_hash` to the docs on
`encode_signing_data_to`.
2022-03-10 07:13:06 +11:00
Tobin Harding aaf587d320 Use correct opcode count
Code comment contains an off-by-one error, update it to the correct
value '61'.
2022-03-10 06:42:09 +11:00
Tobin Harding 146d5e83d1 Improve docs for blockdata::block
Improve the rustdocs for the `blockdata::block` module:

- Use full sentences (capitalisation and full stop)
- Use third person tense instead of imperative
- Improve wording if needed
2022-03-10 06:39:32 +11:00
Tobin Harding f03092c380 Fix erroneous function rustdoc
The returns part of the function docs appears to be stale, remove it.
Improve wording of rustdocs while we are at it.
2022-03-10 06:39:32 +11:00
Tobin Harding 5464848f45 Refactor check_witness_commitment
Currently function contains nested `if` clauses that arguably obfuscate
the code. We can make the code easier to read by pulling out the error
paths and returning them higher up in the function.

Refactor only, no logic changes.
2022-03-10 06:39:04 +11:00
Tobin Harding f4886afa66 Add full stops to docs
Add full stops to all lines of rustdocs in the `blockdata::opcodes`
module.
2022-03-09 13:55:14 +11:00
Tobin Harding f01f047b21 Remove unnecessary newlines
Whitespace only, no code changes.
2022-03-09 13:55:14 +11:00
Tobin Harding 8a1cc2ca77 Improve docs on ClassifyContext
Improve docs on `ClassifyContext` by doing:

- Separate brief doc line from the rest
- Use uniform backticks on opcodes
2022-03-09 13:55:14 +11:00
Tobin Harding ee3b8c267d Order impl_hashencode lines
Put the calls to `impl_hashencode` in the same order, and with the same
whitespace, as the calls to `hash_newtype`. This makes groking the file
easier because its quick to glance down the types and see which ones
implement hashencode (consensus_encode/decode) and which ones do not.
2022-03-09 13:18:48 +11:00
Tobin Harding ac105903cd Flatten the policy module
The policy module contains a single `mod.rs` file, this is unnecessary,
we can simply use `policy.rs` and flatten the module.
2022-03-09 10:20:18 +11:00
Tobin Harding 7638d59fa6 Improve rusntdocs for *_hash_ty methods
Improve the docs by doing:
- Use markdown heading for `Errors` section
- Use 100 character lines
2022-03-08 09:14:20 +11:00
Andrew Ahlers 51fef76129 feat: Add Address.is_related_to_pubkey() 2022-03-06 20:30:22 +01:00
sanket1729 91c5d7192f Change the parameter for control block verification
Changes the API from TweakedPublicKey to XonlyPublicKey. I believe we
introduced TweakedPublicKey to guard against creating address API. This
is confusing because when we want to verify control block we have to
call dangerous_assume_tweak.
This is in true in most cases that the key would be tweaked, but we only
want to guard in while creating a new address. If we want to verify
blocks, we should deal with native X-only-keys regardless of how they
were created
2022-02-28 08:31:20 -08:00
sanket1729 1ec9e87255
Merge rust-bitcoin/rust-bitcoin#842: Separate out merge method into public trait
5e2449922d Separate merge logic out of Map trait (Tobin Harding)

Pull request description:

  Recently we (*cough* Tobin) made the `Map` trait private and neglected
  to add a public API for merging together two PSBTs. Doing so broke the
  `psbt` module.

  Add a public trait `Merge` and implement it for
  `PartiallySignedTransaction` using the code currently in the `merge`
  method of the now private `Map` trait.

  Motivated by https://github.com/rust-bitcoin/rust-bitcoin/pull/841

ACKs for top commit:
  JeremyRubin:
    > ACK 5e24499
  apoelstra:
    ACK 5e2449922d
  sanket1729:
    ACK 5e2449922d. Also verified that the vectors are same of that of BIP174

Tree-SHA512: 79eefe93e870b61231b388aa28a95ee5c8ac06b68910f4ff324569512a79eafe5b86239fd45f54ca7a868cf59dc6301e45d1f046c039a64b2493a8ffcea659fd
2022-02-28 08:30:31 -08:00
Rishabh Singhal fb04cabe1d
Add a method to psbt to compute find sighash type
Fixes #838: Add a utility method to psbt to compute find sighash
type of a given input.
2022-02-25 18:38:19 +05:30
Andrew Poelstra 2c1077e681
Merge rust-bitcoin/rust-bitcoin#829: Don't allow uncompressed public keys without prefix 0x04
c0d36efb8b Don't allow uncompressed public keys without prefix 0x04 (Noah Lanson)

Pull request description:

  Was following #520 and through it was a quick fix that I could do:

  #### Changes:
  - If an uncompressed public key doesn't have prefix 0x04 in `PublicKey::from_slice()`, an error is returned.

  <br>

  I was wondering if `PublicKey::from_str()` should also enforce the same rules, however I have not incuded this in the PR.

  Please let me know if any changes need to be made.

  Thanks

ACKs for top commit:
  Kixunil:
    ACK c0d36efb8b
  apoelstra:
    ACK c0d36efb8b
  sanket1729:
    utACK c0d36efb8b. Not thrilled about the error message expecting len 66, when it can be both 66/130. But can live with it

Tree-SHA512: cfbcd569691c9a7f69ee775ec530605f42e988470a2ff9c28b4c881cec6b259053bb2288818e00b6f6b20316b1fb30fecc0b9a240ebbe7618f202ef6b5efeb9b
2022-02-24 16:50:55 +00:00
Tobin Harding 5e2449922d
Separate merge logic out of Map trait
Recently we (*cough* Tobin) made the `Map` trait private and neglected
to add a public API for combining together two PSBTs. Doing so broke the
`psbt` module.

Pull the merge logic out of the `Map` trait and put it in methods on
each individual type (`Input`, `Output`, `PartiallySignedTransaction`).
Doing so allows for simplification of return types since combining
inputs/outputs never errors.

Use the term 'combine' instead of 'merge' since that is the term used in
BIP 174.
2022-02-23 09:03:16 +00:00
Andrew Poelstra 04787d4867
Merge rust-bitcoin/rust-bitcoin#835: Change Prevouts::All(&[TxOut]) to Prevouts::All(&[&TxOut])
10fedfb3b4 Change Prevouts::All(&[TxOut]) to Prevouts::All(&[Borrow<T>]) (sanket1729)

Pull request description:

  I believe this avoids some allocation of creating a vec of TxOut to
  create a slice incase the data is already available in psbt/other
  methods.

  See #834

ACKs for top commit:
  apoelstra:
    ACK 10fedfb3b4
  Kixunil:
    ACK 10fedfb3b4

Tree-SHA512: 20f69c626b38d6b3c03c8cb370cfad097bbf0bfefff9bb2379c8af3bc94e25d8cc45fc5d69488aeefad58a95470e8f30eb7b400349992a9ebd0d3a13870cba43
2022-02-17 16:56:36 +00:00
sanket1729 10fedfb3b4 Change Prevouts::All(&[TxOut]) to Prevouts::All(&[Borrow<T>])
This avoids some allocation of creating a vec of TxOut to
create a slice incase the data is already available in psbt/other
methods. Facilitates creation of Prevouts from &[TxOut] as well as
&[&TxOut]
2022-02-17 04:45:42 -08:00
sanket1729 4e19973d4e Add a breaking test
This commit can be re-ordered before the fix to see that the test fail
during psbt decoding
2022-02-17 02:48:29 -08:00
sanket1729 69c6eb6173 Bug: Change type of pbst partial sig from secp key to bitcoin key
This changes the type of secp signature from secp256k1::Signature to
bitcoin::PublicKey. Psbt allows storing signatures for both compressed
as well as uncompressed keys. This bug was introduced in #591 while
trying to change the type of BIP32 keys from bitcoin::PublicKey to
secp256k1::PublicKey.
2022-02-16 23:45:35 -08:00
Noah Lanson c0d36efb8b Don't allow uncompressed public keys without prefix 0x04 2022-02-17 08:46:20 +11:00
Andrew Ahlers 79cee4cd31 fix: Error on unsuported addresses in `is_signed_by_address`
Inspired by this comment: https://github.com/rust-bitcoin/rust-bitcoin/pull/684#issuecomment-1012136845
2022-02-09 23:06:22 +01:00
Andrew Ahlers e391ce9939 test: Add a test for incorrect message signature 2022-02-09 22:04:23 +01:00
Andrew Poelstra cb35766979
Merge rust-bitcoin/rust-bitcoin#808: Refactor logical operators
df7bb03a67 Simplify read_scriptbool (Tobin Harding)
4b6e86658d Refactor is_provably_unspendable (Tobin Harding)
e54a2d653b Put && operator at front of line (Tobin Harding)
f5512c4931 Refactor is_p2pkh (Tobin Harding)
373ea89a9a Simplify read_scriptbool (Tobin Harding)
654b2772b8 Add passing unit tests for read_scriptbool (Tobin Harding)

Pull request description:

  In an effort to make the code clearer and more explicit, do various refactorings around logical operators. Each done as a separate patch to ease review and limit scope of discussion.

  Based on review of https://github.com/rust-bitcoin/rust-bitcoin/pull/806

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

Tree-SHA512: 06460979d492eb38cefc147397338b7fd95320c66ce8e8b4f8e2b454bb35721ce308413690a0618bd19d695df56175646d4d0c619388c0268f7fd35d5a7b6a3d
2022-01-26 13:14:16 +00:00
Tobin Harding df7bb03a67 Simplify read_scriptbool
Simplify `read_scriptbool` by doing:

- Use `split_last` to get at the last element
- Mask the last byte against ^0x80 instead of using two equality
  statements
2022-01-26 16:52:41 +11:00
Riccardo Casatta 22aeaef52b
Use write_all instead of write
write() could write only a part of the given buffer, the caller should
check the numbers of byte written (which is what write_all does)
2022-01-25 15:09:21 +01:00
Tobin Harding 4b6e86658d Refactor is_provably_unspendable
Refactor with the aim of making the code easier to read. Code path is
covered by current unit tests.

Refactor only, no logic changes.
2022-01-25 10:21:06 +11:00
Tobin Harding e54a2d653b Put && operator at front of line
In an effort to make code containing multi-line logical AND clearer to
read put the operator at the start of the line.
2022-01-25 10:21:06 +11:00
Tobin Harding f5512c4931 Refactor is_p2pkh
Refactor with the aim of simplifying `is_p2kh`. This function is covered
sufficiently by current unit tests.

Refactor only, no logic changes.
2022-01-25 10:16:56 +11:00
Tobin Harding 373ea89a9a Simplify read_scriptbool
Refactor and simplify the logical operators in `read_scriptbool`.

Refactor only, no logic changes.
2022-01-25 10:16:56 +11:00
Tobin Harding 654b2772b8 Add passing unit tests for read_scriptbool
In preparation for refactoring `read_scriptbool` add passing unit
tests.
2022-01-25 10:16:54 +11:00
sanket1729 325e0ccf51
Merge rust-bitcoin/rust-bitcoin#800: Use fn name to_ instead of into_
151173821b Use fn name to_ instead of into_ (Tobin Harding)

Pull request description:

  Rust convention is to use `to_` for conversion methods that convert from
  an owned type to an owned `Copy` type. `into_` is for owned to owned
  non-`Copy` types.

  Re-name conversion methods that use `into_` for `Copy` types to use
  `to_`, no need to deprecate these ones because they are unreleased.

  **Note to maintainers**

  This is similar in concept to #798 but only touches new code introduced in this release. Has been labelled 'RC fix' for that reason. Please feel free to remove the label if you disagree.

  From the docs: https://rust-lang.github.io/api-guidelines/naming.html

  <h2><a class="header" href="https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv" id="ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv">Ad-hoc conversions follow <code>as_</code>, <code>to_</code>, <code>into_</code> conventions (C-CONV)</a></h2>
  <p>Conversions should be provided as methods, with names prefixed as follows:</p>

  Prefix | Cost | Ownership
  -- | -- | --
  as_ | Free | borrowed -> borrowed
  to_ | Expensive | borrowed -> borrowed
  | | | borrowed -> owned (non-Copy types)
  | | | owned -> owned (Copy types)
  into_ | Variable | owned -> owned (non-Copy types)

ACKs for top commit:
  Kixunil:
    ACK 151173821b
  apoelstra:
    ACK 151173821b
  sanket1729:
    ACK 151173821b

Tree-SHA512: 4bb97e4fb78beda0fd1ec9482d24ef0f4ade6d3689f5c1bcf2208fa2df3195962522fa5d5ac700e6d4d5ff2096a20b2a0ad51784909a3c12405762aa08d1ced2
2022-01-21 08:00:31 +05:30
Casey Rodarmor 94dd57de12 Add subsidy halving interval constant 2022-01-20 08:52:46 -08:00
Andrew Poelstra b327386cc8
Merge rust-bitcoin/rust-bitcoin#799: Use 4 character indentation
8d780e8172 Use 4 character indentation (Tobin Harding)

Pull request description:

  Bizarrely the impl block for `EcdsaSigHashType` uses 5 character indentation.

  Use 4 character indentation as is typical.

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

Tree-SHA512: 4b0789b4ef6f3e817fcbe002ca0d9ee4fca4a3dc70bbeaebae745a2f0497eb072f3c3876805d97ff58eb8a9464ea4443f23cea0c95bfd052e382caa9f76b515d
2022-01-19 18:57:48 +00:00
Riccardo Casatta 1f0810ad6e
Merge rust-bitcoin/rust-bitcoin#790: Re-export psbt module from root level
b138428df7 Re-export public map types from root level (Tobin Harding)

Pull request description:

  We currently have the `map` module private but containing a bunch of types that are needed in the public API (specifically in a `PartiallySignedTransaction`).

  To give access to them re-export the `util::psbt` module at the root level.

  Found while testing `master` with `rust-miniscript`.

ACKs for top commit:
  sanket1729:
    utACK b138428df7
  Kixunil:
    ACK b138428df7
  RCasatta:
    ACK b138428df7
  dr-orlovsky:
    ACK b138428df7

Tree-SHA512: 36fc8595164c4975abdadb6c8149ef27686a2d681a1815379f91b1bd36f8a56ceaa7faed5979ba6869823684790721a16a0c41e662c6227a09cd0ba576a0a181
2022-01-19 12:18:14 +01:00
Tobin Harding 151173821b Use fn name to_ instead of into_
Rust convention is to use `to_` for conversion methods that convert from
an owned type to an owned `Copy` type. `into_` is for owned to owned
non-`Copy` types.

Re-name conversion methods that use `into_` for `Copy` types to use
`to_`, no need to deprecate these ones because they are unreleased.
2022-01-19 14:59:18 +11:00
Tobin Harding 8d780e8172 Use 4 character indentation
Bizarrely the impl block for `EcdsaSigHashType` uses 5 character
indentation.

Use 4 character indentation as is typical.
2022-01-19 14:30:36 +11:00
Andrew Poelstra 64451a2144
Merge rust-bitcoin/rust-bitcoin#794: Refactor use map_err
9f848472e4 Refactor use map_err (wim-web)

Pull request description:

  issue: https://github.com/rust-bitcoin/rust-bitcoin/issues/793

  change to using map_err

ACKs for top commit:
  Kixunil:
    ACK 9f848472e4
  apoelstra:
    ACK 9f848472e4

Tree-SHA512: 93dac16463bf84825f764f3ef81833c27722a52f56737d30f14160d070959ad13bbfdf5f3c4871b961ce05fa9f75ed36acbacaa40ff6ba3bbf449b9c9173c0c7
2022-01-18 20:55:31 +00:00
wim-web 9f848472e4 Refactor use map_err 2022-01-18 13:20:53 +09:00
Tobin Harding b138428df7 Re-export public map types from root level
We currently have the `map` module private but containing a bunch of
types that are needed in the public API (specifically in a
`PartiallySignedTransaction`).

Re-export the publicly required types to the `psbt` module and then
again at the root level of `rust-bitcoin` as we do for other types.
2022-01-18 12:56:47 +11:00
Andrew Poelstra 8acdb1ab64
Merge rust-bitcoin/rust-bitcoin#786: Fix unused arg in PSBT impl_psbt_get_pair macro
1b77e3609c Fix unused arg in PSBT impl_psbt_get_pair macro (Dr Maxim Orlovsky)

Pull request description:

  Closes #754

ACKs for top commit:
  apoelstra:
    ACK 1b77e3609c
  RCasatta:
    utACK 1b77e3609c
  Kixunil:
    ACK 1b77e3609c

Tree-SHA512: 339aae0a9b6adef34bc6bca27ea19eb89205c4b4694e3a95721590696b7eefbfded9294fd9101a66110b4bdfb69da7a7ef97a8e8321d9e05b3777c3fa9afff7c
2022-01-17 19:54:29 +00:00
Dr. Maxim Orlovsky 6cca59296e
Merge rust-bitcoin/rust-bitcoin#791: Remove trailing spaces from 'address.rs'
4284c2be3b Remove trailing spaces from 'address.rs' (Roman Zeyde)

Pull request description:

ACKs for top commit:
  dr-orlovsky:
    ACK 4284c2be3b
  Kixunil:
    ACK 4284c2be3b

Tree-SHA512: 62fb1ea934ed0e089766db84d1a9a218a922272555d89081f5e47ac3a9d236474c8218e1873251961a9562405817e16f2f1ea84731f330813b21898015c1cabd
2022-01-17 12:15:48 +02:00
Dr. Maxim Orlovsky d5686ee01d
Merge rust-bitcoin/rust-bitcoin#776: Change EcdsaSig hash type deser in psbt
abe52f681b Cleanup/Dedup psbt (De)Serialization code (sanket1729)
fbd86dcf63 Update documentation of EcdsaSig::from_slice (sanket1729)
85009a7b50 Update documentation of from_u32_consensus (sanket1729)
0fed04e2d5 Change EcdsaSig hash type deser (sanket1729)

Pull request description:

  Changes the parsing behavior in PSBT on non-standard sighash types to give an explicit error, rather than silently mangling the parsed value

ACKs for top commit:
  dr-orlovsky:
    ACK abe52f681b
  apoelstra:
    ACK abe52f681b
  Kixunil:
    ACK abe52f681b

Tree-SHA512: 1d5dbe3aa5885ca16649cf8ea05a7476e8dd977dd870b79358d97a3ce383bee93754d2b88163e7db3792cdc4b9cb867356409c8eea4e110877577ad196ba0786
2022-01-17 12:08:18 +02:00
Roman Zeyde 4284c2be3b Remove trailing spaces from 'address.rs' 2022-01-17 10:03:17 +02:00
Dr Maxim Orlovsky 1b77e3609c Fix unused arg in PSBT impl_psbt_get_pair macro
Closes #754
2022-01-16 11:59:29 +01:00
sanket1729 093f8b612d
Merge rust-bitcoin/rust-bitcoin#752: Make Map trait private
dfd8924398 Remove insert_pair from Map trait (Tobin Harding)
ad75d5181f Make Map trait private to psbt module (Tobin Harding)
53225c0a6e Improve docs in map module (Tobin Harding)
92059c2841 Add full stops to rustdocs (Tobin Harding)
11c046b707 Refactor match arms (Tobin Harding)
e6af569490 Move imports to top of file (Tobin Harding)

Pull request description:

  The `Map` method `insert_pair` is never called for `PartiallySignedTransaction`. Separate the method into its own trait (`Insert`) and delete dead code. The dead code contains the alleged bug in #576.

  - Patch 1: Preparatory cleanup
  - Patch 2: Preparatory refactor
  - Patch 3 and 4: Improve docs in the module that this PR touches
  - Patch 5: Make `Map` trait private to the `psbt` module
  - ~Patch 6: Make `concensus_decode_global` method into a function~
  - Patch ~7~ 6: Pull `insert_pair` method out of `Map` trait into newly create `Insert` trait

  Resolves: https://github.com/rust-bitcoin/rust-bitcoin/issues/576

  (Title of PR is `Make Map trait private` because that is the API break.)

ACKs for top commit:
  dr-orlovsky:
    ACK dfd8924398
  apoelstra:
    ACK dfd8924398

Tree-SHA512: 1a78294bc8a455552d93caf64db697f886345ba979f574abad55820415958fee1c2dd16945f4eafdbe542fa202cb7e08618aa137ec7ee22b3c9dac5df0328157
2022-01-16 08:49:27 +05:30
sanket1729 abe52f681b Cleanup/Dedup psbt (De)Serialization code 2022-01-15 06:15:54 +05:30
sanket1729 fbd86dcf63 Update documentation of EcdsaSig::from_slice 2022-01-15 06:15:14 +05:30
sanket1729 85009a7b50 Update documentation of from_u32_consensus 2022-01-15 06:15:14 +05:30
sanket1729 0fed04e2d5 Change EcdsaSig hash type deser 2022-01-15 06:15:10 +05:30
sanket1729 d1f051c95a
Merge rust-bitcoin/rust-bitcoin#757: Minimally-invasive separation of bitcoin keys from ECDSA signature types
8a993e8a58 Properly deprecate util::ecdsa key re-exports (Dr Maxim Orlovsky)
bcb8932ccf Re-org keys and ecdsa mods - pt.3 (Dr Maxim Orlovsky)
d1c2213d3b Re-org keys and ecdsa mods - pt.2 (Dr Maxim Orlovsky)
b9170162d5 Re-org keys and ecdsa mods - pt.1 (Dr Maxim Orlovsky)
2d9de78725 Re-export all key types under `util::key`. Deprecate other exports. (Dr Maxim Orlovsky)

Pull request description:

  This PR tries to do a minimally-invazive separation of signature- and key-related types, previously mixed in a single `util::ecdsa` module.

  Rationale: bitcoin key types are not specific for signature algorithm. See discussion at #588.

  This PR became possible after we moved on new `secp256k1` version exposing `XonlyPublicKey` type, since now all key types may co-exist in a single module under different names

  The PR goal is achieved through
  - Renaming ecdsa mod into private ec module such that the code is not copied and diff size is small;
  - Introducing dummy ecdsa mod back in the next commit and re-exporiting only signature types from internal `ec` mod in it;
  - Re-exporting all key types under `key` module, removing previous depreciation message for bitcoin keys.

ACKs for top commit:
  apoelstra:
    ACK 8a993e8a58
  sanket1729:
    utACK 8a993e8a58

Tree-SHA512: 9f71edaa2cf4cdab4b239cb1d57576e2ba0fc3c2ec0ea19ae232005967b9400da6ded992b33d10b190ca617a66dca9b99be430bc5058a064f0be1489723c4a3a
2022-01-15 05:52:53 +05:30
Tobin Harding dfd8924398 Remove insert_pair from Map trait
The method implementation of `insert_pair` is currently not used for
`PartiallySignedTransaction`. Having an implementation available is
deceiving.

Delete the unused `insert_pair` code from
`PartiallySignedTransaction` (dead code). Make the `insert_pair` methods
from `Input` and `Output` be standalone functions.
2022-01-15 10:04:15 +11:00
Tobin Harding ad75d5181f Make Map trait private to psbt module
The `Map` trait has been deemed confusing and not that useful to users
of the library, we still use it internally within the `psbt` module
though so make it visible only in `psbt` and `psbt::map`.
2022-01-15 10:03:47 +11:00
Tobin Harding 53225c0a6e Improve docs in map module
Improve the function rustdocs in the `psbt::map` module by:

- using third person tense as is idiomatic in the Rust ecosystem
- using rustdoc `///` not code comments `//` for methods
- Use `# Return` section for documenting return values

Done for this module only as part of a PR fixing code within this
module.
2022-01-15 10:03:43 +11:00
Tobin Harding 92059c2841 Add full stops to rustdocs
Mildly improve the docs by adding full stops to every rustdoc comment.
2022-01-15 10:03:38 +11:00
Tobin Harding 11c046b707 Refactor match arms
Refactor the match arms to make the code around the key used for map look
up easier read.

Refactor only, no logic changes.
2022-01-15 10:02:41 +11:00
Tobin Harding e6af569490 Move imports to top of file
These imports are unusually placed, from the code comment it seems the
reason is stale.

Move imports to top of file as is typical.
2022-01-15 10:02:38 +11:00
KaFai Choi 8fef869c15
repalce unncessary extra closure with function pointer in starts_with_uppercase closure inside Denomination from_str 2022-01-14 21:07:43 +07:00
Dr. Maxim Orlovsky b165b8da05
Merge rust-bitcoin/rust-bitcoin#768: add nano and pico BTC to Denomination enum
40f38b3edc enforce strict SI(treat capital of m, u, n, p as invalid) in parsing amount denomiation. add disallow_unknown_denomination test (KaFai Choi)
e80de8b1ee add nano and pico BTC to Donomination enum (KaFai Choi)

Pull request description:

  Close [741](https://github.com/rust-bitcoin/rust-bitcoin/issues/741)

ACKs for top commit:
  Kixunil:
    ACK 40f38b3edc
  apoelstra:
    ACK 40f38b3edc
  dr-orlovsky:
    Changing review to ACK 40f38b3edc since it was my misunderstanding and not a bug

Tree-SHA512: 4cc380b8e7403e37e7993e25848b25d74c610d4e9fe274526c613d4b3e2a9f6677c7df52310fc1cab6f1d629d9529ff9f5a2efa41d9e07eab62d0989780ae3a4
2022-01-14 11:22:57 +02:00
Dr Maxim Orlovsky 8a993e8a58 Properly deprecate util::ecdsa key re-exports 2022-01-14 09:45:22 +01:00
Dr Maxim Orlovsky bcb8932ccf Re-org keys and ecdsa mods - pt.3 2022-01-14 01:35:48 +01:00
Dr Maxim Orlovsky d1c2213d3b Re-org keys and ecdsa mods - pt.2 2022-01-14 01:35:48 +01:00
Dr Maxim Orlovsky b9170162d5 Re-org keys and ecdsa mods - pt.1
This commit tries to achieve separation of signature- and key-related types, previously mixed in a single ECDSA module.

Rationale: bitcoin key types are not specific for signature algorithm.

This is achieved through
- Remove key mod with its content moved to ecdsa mod
- Re-export keys under key module in util mod - to make git generate diff for the rename of ecdsa mod in the next commit correctly.
2022-01-14 01:35:48 +01:00
Dr Maxim Orlovsky 2d9de78725 Re-export all key types under `util::key`. Deprecate other exports. 2022-01-14 01:35:48 +01:00
sanket1729 ebdeed086e Cleanup imports
We do not want to imports from within the lib and external of lib in the
same line
2022-01-14 05:39:17 +05:30
sanket1729 382c8f9e4f Introduce PsbtSigHashType 2022-01-14 05:39:17 +05:30
Dr Maxim Orlovsky 62a27a51e2 Document that serde impl of LeafVersion uses u8 in consensus encoding
Closes #764
2022-01-13 17:53:50 +01:00
Dr Maxim Orlovsky 73e6ce4e53 Re-export Witness at crate level. Closes #770 2022-01-13 17:51:01 +01:00
Dr Maxim Orlovsky 6364ebd927 Code style fixups to taproot key functions 2022-01-13 17:48:13 +01:00
Dr Maxim Orlovsky 7514f2ca18 Tweaked -> untweaked keys conversions 2022-01-13 17:40:27 +01:00
Andrew Poelstra 907b3a7a6a
Merge rust-bitcoin/rust-bitcoin#775: Issue #394 - Refactor Block::merkle_root()
cd2435c807 Change deprecated version to 0.28. Remove redundant Block::merkle_root() logic. (Nils Loewen)
05788285f5 Issue #394 - Refactor Block::merkle_root() to Block::compute_merkle_root() and deprecate Block::merkle_root(). (Nils Loewen)

Pull request description:

  Refactor `Block::merkle_root()` to `Block::compute_merkle_root()` and deprecate `Block::merkle_root()`.

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

Tree-SHA512: 820d85d5a25b7316046d5df8e3ab1e8cd57f72c7fa63f0d6826b965c5da3ef1bfacd4704180810aa8c0a7a224dcd62f398a2cf93fc344e2b0d52d9d7024c6c27
2022-01-13 14:32:30 +00:00
KaFai Choi 40f38b3edc
enforce strict SI(treat capital of m, u, n, p as invalid) in parsing amount denomiation. add disallow_unknown_denomination test 2022-01-13 20:27:41 +07:00
sanket1729 7d62277f83
Merge rust-bitcoin/rust-bitcoin#696: Taproot tweaks generalization & KeyPair support
7405836411 Fix warning about deprecated method use (Dr Maxim Orlovsky)
f39b1300fa CI: do not fail fast (Dr Maxim Orlovsky)
f77c57195a Making Script method new_* names more consistent (Dr Maxim Orlovsky)
91b68a468d Taproot-related methods for Script type (Dr Maxim Orlovsky)
599c5f9488 Generalizing taproot key tweaking for KeyPairs (Dr Maxim Orlovsky)

Pull request description:

  * Adds taproot-related methods to `Script`
  * Fixes API for existing taproot methods
  * Generalizes `TapTweak` trait to work with both public keys and key pairs

  ~~UPD: PR is pending https://github.com/rust-bitcoin/rust-secp256k1/pull/342~~

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

Tree-SHA512: 4a76dfffa1452baadc15e19812831ef9d2e66794c090a8fc123388d7119b2c8a1f0420ce723ad22e01683c8198711fe62e0cdf00c9ad2d2974606383baaf1cb0
2022-01-13 10:06:30 +05:30
Nils Loewen cd2435c807
Change deprecated version to 0.28. Remove redundant Block::merkle_root() logic. 2022-01-12 16:23:35 -08:00
Nils Loewen 05788285f5
Issue #394 - Refactor Block::merkle_root() to
Block::compute_merkle_root() and deprecate Block::merkle_root().
2022-01-12 13:37:23 -08:00
Andrew Poelstra bc9388e24a
Merge rust-bitcoin/rust-bitcoin#774: Change type of final script witness to Witness from Vec<Vec<u8>>
9a8ab3f3ff Change type of final script witness to Witness from Vec<Vec<u8>> (sanket1729)

Pull request description:

  Doing this would certainly help APIs downstream that operate on &Witness because they would not conversion from &Vec<Vec<u8>> to &Witness.

ACKs for top commit:
  Kixunil:
    ACK 9a8ab3f3ff
  RCasatta:
    ACK 9a8ab3f3ff
  dr-orlovsky:
    ACK 9a8ab3f3ff
  apoelstra:
    ACK 9a8ab3f3ff

Tree-SHA512: 647e18d254a51d6216a0122407146e8bc1d39504e76c1e0e746f740cec7cda587455b61d4cdadc3c59b1cf03eba87000de35fbde645a30fb166a84847ba101b2
2022-01-11 16:05:59 +00:00
sanket1729 9a8ab3f3ff Change type of final script witness to Witness from Vec<Vec<u8>> 2022-01-11 21:11:18 +05:30
Dr Maxim Orlovsky 7405836411 Fix warning about deprecated method use 2022-01-11 16:10:29 +01:00
Dr Maxim Orlovsky f77c57195a Making Script method new_* names more consistent 2022-01-11 16:10:29 +01:00
Dr Maxim Orlovsky 91b68a468d Taproot-related methods for Script type 2022-01-11 16:10:29 +01:00
Dr Maxim Orlovsky 599c5f9488 Generalizing taproot key tweaking for KeyPairs 2022-01-11 16:09:32 +01:00
Andrew Poelstra d9bc48f12f
Merge rust-bitcoin/rust-bitcoin#771: Add Witness::new()
5d27c15717 Add Witness::new() (sanket1729)

Pull request description:

  Rebasing rust-miniscript on rust-bitcoin master. Expect more trivial PRs :)

ACKs for top commit:
  dr-orlovsky:
    ACK 5d27c15717
  apoelstra:
    ACK 5d27c15717

Tree-SHA512: f1b6b1bba1ba6f153416be514b53d64bb6ba0c1cc1f182857f5010a048004f884d5c36ef96f6e27837bf682d781e3a4375bf2303e1f53685019406e88e192a03
2022-01-11 14:49:14 +00:00
KaFai Choi e80de8b1ee
add nano and pico BTC to Donomination enum 2022-01-11 19:23:45 +07:00
Dr Maxim Orlovsky eb09019720 Rename inner key field in PrivateKey and PublicKey
Closes #532
2022-01-11 08:39:52 +01:00
sanket1729 e4d5039a86
Merge rust-bitcoin/rust-bitcoin#591: PSBT BIP32 keys using to Secp256k1 keys instead of bitcoin ECDSA
a6e8f581db PSBT BIP32 keys moved to Secp256k1 from bitcoin ECDSA (Dr Maxim Orlovsky)

Pull request description:

  Fourth step in implementation of Schnorr key support after #588. This PR is a follow-up to non-API breaking #589 and API-breaking #590, which must be reviewed and merged first. ~~(The current PR includes all commits from #589 and #590, which should be reviewed there. The only commit specific to this PR is b8105e95dc8651626b783403ca060f7d32d21144)~~

  UPDATE: All related PRs are merged now and this PR is ready for the review

  PR description:
  While PSBT BIP174 does not specify whether uncompressed keys are supported in BIP32-related fields, from BIP32 it follows that it is impossible to use uncompressed keys within the extended keys.  This PR fixes this situation and is a companion to BIP174 PR clarifying key serialization: https://github.com/bitcoin/bips/pull/1100

ACKs for top commit:
  apoelstra:
    ACK a6e8f581db
  sanket1729:
    ACK a6e8f581db. Not sure which order to merge since there are many ready PRs which that would break each other.

Tree-SHA512: 198ba646bbce1949b255a54a97957d952acdad8b7f9580be123116c0f44d773e6d90e0cac0d5993ec9a6b3328aa43aced0908522817861585877c50008fec835
2022-01-11 12:42:53 +05:30
sanket1729 5d27c15717 Add Witness::new()
I think it is more natural to write Wintess::new() followed by Witness::push()
then Witness::default(). In any case, there is no harm in having additional constructors.
2022-01-11 07:37:20 +05:30
Dr Maxim Orlovsky a6e8f581db PSBT BIP32 keys moved to Secp256k1 from bitcoin ECDSA
Fourth step in implementation of Schnorr key support after #588.

While PSBT BIP174 does not specify whether uncompressed keys are supported in BIP32-related fields, from BIP32 it follows that it is impossible to use uncompressed keys within the extended keys.  This PR fixes this situation and is a companion to BIP174 PR clarifying key serialization: https://github.com/bitcoin/bips/pull/1100
2022-01-10 10:16:57 +01:00
KaFai Choi 9835736ef5
wrap u8 and LeafVersion in backticks and square bracket in doc 2022-01-10 15:09:44 +07:00
sanket1729 d82afc6ef5
Merge rust-bitcoin/rust-bitcoin#761: Taproot trivial post-merge fixups
7f06e91a93 LowerHex and UpperHex implementations for LeafVersion (Dr Maxim Orlovsky)
6a3f3aabaf Inverse alternative formatting for LeafVersion type (Dr Maxim Orlovsky)
bec6694233 Fix docs on error conditions in LeafVersion::from_consensus (Dr Maxim Orlovsky)
7c28b47451 LowerHex and UpperHex implementations for FutureLeafVersion (Dr Maxim Orlovsky)

Pull request description:

  Trivial post-merge fixups from review comments in #718

ACKs for top commit:
  Kixunil:
    ACK 7f06e91a93
  sanket1729:
    ACK 7f06e91a93

Tree-SHA512: d94c4bd3d0b466287c8965103f74ecaba185d14c13b6c3f37d9fbe194343b3fc902fd2c7716554ad01fe28ff89cda933df199b7e8388a3fa6097028caf62522b
2022-01-10 04:18:48 +05:30
sanket1729 476eed7f2f
Merge rust-bitcoin/rust-bitcoin#590: Taproot: BIP32 extended keys using Scep256k1 keys instead of bitcoin ECDSA
cf0c48cc86 Improve Debug for PrivateKey (Dr Maxim Orlovsky)
b65a6ae49b Test for extended private key keypair generation  f5875a (Dr Maxim Orlovsky)
e6a3d603c9 BIP32 extended key `to_ecdsa()` and `to_schnorr()` methods (Dr Maxim Orlovsky)
b72f56c4ae BIP32 extended keys are using Scep256k1 keys instead of bitcoin ECDSA (Dr Maxim Orlovsky)

Pull request description:

  This is third step required to introduce Schnorr key support according to #588. This PR starts API-breaking changes and is follow-up to non-API breaking #589, which is already merged.

  PR rationale: BIP32 does not support uncompressed keys and using type with compression flag was a mistake

ACKs for top commit:
  apoelstra:
    ACK cf0c48cc86
  sanket1729:
    ACK cf0c48cc86. #757 might need rework after this

Tree-SHA512: 6356a65004e7517256bacbf9aaeb69a22fd8536b341e567c5c4e819288e1105d083fe12ac0641404c407c97acf039bdc525f8e02b1b594a6cdda90106f3b1bdc
2022-01-10 03:46:05 +05:30
Dr Maxim Orlovsky 7f06e91a93 LowerHex and UpperHex implementations for LeafVersion 2022-01-09 20:52:38 +01:00
Dr Maxim Orlovsky 6a3f3aabaf Inverse alternative formatting for LeafVersion type 2022-01-09 20:50:22 +01:00
Dr Maxim Orlovsky bec6694233 Fix docs on error conditions in LeafVersion::from_consensus 2022-01-09 20:48:00 +01:00
Dr Maxim Orlovsky 7c28b47451 LowerHex and UpperHex implementations for FutureLeafVersion 2022-01-09 20:46:51 +01:00
Andrew Poelstra 8e9f99b620
Merge rust-bitcoin/rust-bitcoin#718: Converting LeafVersion into an enum
ef8a3a839e Introduce FutureLeafVersion (Dr Maxim Orlovsky)
b028385a72 Improve docs in LeafVersion (Dr Maxim Orlovsky)
839c022f29 Make serde for LeafVersion to have byte representation (Dr Maxim Orlovsky)
67b8db05a8 Converting LeafVersion into an enum (Dr Maxim Orlovsky)
2405417432 Use TAPROOT_ANNEX_PREFIX in sighash module (Dr Maxim Orlovsky)

Pull request description:

  The original `LeafVersion` implementation was just a newtype around `u8`. I think that having enum explicitly listing consensus script implementation rules may be more beneficial in terms of both code readibility and future use of multiple script types, where `LeafVersion` may operate as a context object provided to `Script` to specify interpretation rules for particular op codes.

ACKs for top commit:
  Kixunil:
    ACK ef8a3a839e
  sanket1729:
    crACK ef8a3a839e. Waiting a day to let others complete review before merging.
  apoelstra:
    ACK ef8a3a839e

Tree-SHA512: 3356d2b9b00cf904edfece26d26ffbc646ba74446cc23ec4b2b4026ed50861285802f077226e30ba8fed466f68f8e8556c729ce48cb38581b1d95a02a6fde9cf
2022-01-09 15:26:05 +00:00
Dr Maxim Orlovsky cf0c48cc86 Improve Debug for PrivateKey 2022-01-09 07:17:10 +01:00
Dr Maxim Orlovsky b65a6ae49b Test for extended private key keypair generation f5875a 2022-01-09 07:17:06 +01:00
Dr Maxim Orlovsky e6a3d603c9 BIP32 extended key `to_ecdsa()` and `to_schnorr()` methods 2022-01-09 07:17:02 +01:00
Dr Maxim Orlovsky b72f56c4ae BIP32 extended keys are using Scep256k1 keys instead of bitcoin ECDSA
According to #588, BIP32 does not support uncompressed keys and using type with compression flag is a mistake
2022-01-09 07:16:49 +01:00
Dr Maxim Orlovsky ef8a3a839e Introduce FutureLeafVersion 2022-01-08 23:40:21 +01:00
Dr Maxim Orlovsky 14ace92666 Fix SchnorrSig type references in PSBT serialization macros 2022-01-08 16:29:45 +01:00
Dr Maxim Orlovsky b028385a72 Improve docs in LeafVersion 2022-01-07 22:06:17 +01:00
Dr Maxim Orlovsky 839c022f29 Make serde for LeafVersion to have byte representation 2022-01-07 22:04:41 +01:00
Dr Maxim Orlovsky 2b530000d3 Use EcdsaSig in PSBT partial signatures instead of Vec<u8> 2022-01-07 21:57:42 +01:00
Dr Maxim Orlovsky 141dbbd1b9 Add serde impl for EcdsaSig 2022-01-07 21:57:42 +01:00
Dr Maxim Orlovsky c92057d98f PSBT serialize/deserialize impl for EcdsaSig type 2022-01-07 21:57:42 +01:00
Dr Maxim Orlovsky 0af1c3f320 Add Display and FromStr for EcdsaSig 2022-01-07 21:57:11 +01:00
Dr Maxim Orlovsky daf0eacf3d Improve NonStandardSigHashType 2022-01-07 21:49:05 +01:00
Dr Maxim Orlovsky c36a3da6f0 Add EcdsaSig::sighash_all convenience constructor 2022-01-07 21:49:05 +01:00
Andrew Poelstra f332a1967e
Merge rust-bitcoin/rust-bitcoin#750: Use `test_data` for big objects, add big block for benchmarking
247a14f4c3 Use test big block for bench_stream_reader instead of making one (Riccardo Casatta)
b92dfbb63f exclude test_data when publishing the crate (Riccardo Casatta)
f5a9681a2a include a big block in test_data, use it for ser/de benchmark (Riccardo Casatta)
09dada55d6 Move bip158 test vectors to test_data (Riccardo Casatta)
06d1a820c3 Remove testnet block hex from tests, use test_data with include_bytes! (Riccardo Casatta)

Pull request description:

  In the first two commits I moved some data from source files to the newly introduced `test_data` dir, including it with `include_[str|bytes]!` macro.

  The second-to-last commit introduces a big block in test_data which is very handy in ser/de benchmark (I used it for #672) because with smaller blocks you may not notice performance improvements.

  Since I don't want to pollute the package the last commit excludes the `test_data` dir from the published package. I think it's fine to do it because dependent packages don't run dependencies tests.

ACKs for top commit:
  apoelstra:
    ACK 247a14f4c3
  Kixunil:
    tACK 247a14f4c3

Tree-SHA512: a2beb635b0a358737d0b57d3e7205b1ddf87652b9a8c889ce63e2867659a8eaf7e43a5b87a453345d56d953745913f40b58596f449e5fbc87340e0dd2aef0727
2022-01-07 20:22:02 +00:00
Dr Maxim Orlovsky 67b8db05a8 Converting LeafVersion into an enum 2022-01-07 20:28:36 +01:00
Dr Maxim Orlovsky 2405417432 Use TAPROOT_ANNEX_PREFIX in sighash module 2022-01-07 20:27:34 +01:00
sanket1729 91470f56c8 Uncomment sighash test
We can check tweak add priv key with latest secp
2022-01-07 04:45:40 +05:30
sanket1729 2178c7367c Update to secp256k1 0.21.2 2022-01-07 04:45:40 +05:30
Riccardo Casatta 9e7bb0967c
Merge rust-bitcoin/rust-bitcoin#753: Put rustdocs above attributes
533120899e Put rustdocs above attributes (Tobin Harding)

Pull request description:

  (Trivial / Very low priority PR)

  Rust idiomatic style is to put the rustdoc _above_ any attributes on types, functions, etc.

  Audit the codebase and move comments/attributes to the correct place. Add a trailing full stop at times to neaten things up a little extra.

  Done after discussion [here](https://github.com/rust-bitcoin/rust-secp256k1/pull/353#discussion_r778393138)

ACKs for top commit:
  Kixunil:
    ACK 533120899e
  RCasatta:
    ACK 533120899e

Tree-SHA512: 7cd00dc46de813cbe3f96417bb4b13980064e10110b421224496c8b64bbe87b61b6c757cc621fde1d05754be6ecdc08acdb51fd8978e3f820d2d93f7104062d1
2022-01-06 14:09:23 +01:00
Riccardo Casatta 247a14f4c3
Use test big block for bench_stream_reader instead of making one 2022-01-06 13:55:50 +01:00
Riccardo Casatta f5a9681a2a
include a big block in test_data, use it for ser/de benchmark 2022-01-06 13:48:02 +01:00
Riccardo Casatta 09dada55d6
Move bip158 test vectors to test_data 2022-01-06 13:47:58 +01:00
Riccardo Casatta 06d1a820c3
Remove testnet block hex from tests, use test_data with include_bytes! 2022-01-06 13:47:51 +01:00
Riccardo Casatta 9c3a27a326
Merge rust-bitcoin/rust-bitcoin#680: Deprecate `StreamReader`
e860333bf3 Fix typos (Riccardo Casatta)
9189539715 Use BufReader internally in StreamReader to avoid performance regression on existing callers (Riccardo Casatta)
5dfb93df71 Deprecate StreamReader (Riccardo Casatta)
9ca6c75b18 Bench StreamReader (Riccardo Casatta)

Pull request description:

  `StreamReader` performance is extremely poor in case the object decoded is "big enough" for example a full Block.

  In the common case, the buffer is 64k, so to successfully parse a 1MB block 16 decode attempts are made.
  Even if a user increases the buffer size, `read` is not going to necessarily fill the buffer, as stated in the doc https://doc.rust-lang.org/stable/std/io/trait.Read.html#tymethod.read. In my tests, the reads are 64kB even with a 1MB buffer.

  I think this is the root issue of the performance issue found in electrs in https://github.com/romanz/electrs/issues/547 and they now have decided to decode the TCP stream with their own code in cd0531b8b7 and 05e0221b8e.

  Using directly `consensus_encode` seems to make more sense (taking care of using `BufRead` if necessary) so the `StreamReader` is deprecated

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

Tree-SHA512: a15a14f3f087be36271da5008d8dfb63866c9ddeb5ceb0e328b4a6d870131132a8b05103f7a3fed231f5bca099865efd07856b4766834d56ce2384b1bcdb889b
2022-01-06 13:28:32 +01:00
Tobin Harding 533120899e Put rustdocs above attributes
Rust idiomatic style is to put the rustdoc _above_ any attributes on
types, functions, etc.

Audit the codebase and move comments/attributes to the correct place.
Add a trailing full stop at times to neaten things up a little extra.
2022-01-06 13:04:47 +11:00
Riccardo Casatta e860333bf3
Fix typos 2022-01-05 09:39:57 +01:00
Riccardo Casatta 9189539715
Use BufReader internally in StreamReader to avoid performance regression on existing callers 2022-01-04 10:31:45 +01:00
sanket1729 92ee5a7e5f Test BIP341 sighash code 2022-01-01 04:12:46 +05:30
Riccardo Casatta 5dfb93df71
Deprecate StreamReader
StreamReader before this commit is trying to repeatedly parse big object like
blocks at every read, causing useless overhead.
consensus_encode deal with partial data by simply blocking.

After this changes it doesn't look what remain of the StreamReader is really giving
value, so it's deprecated
2021-12-31 10:44:14 +01:00
Dr. Maxim Orlovsky 670e808c17
Merge rust-bitcoin/rust-bitcoin#681: Add support for taproot psbt fields BIP 371
7d982fa9a2 Add all tests from BIP 371 (sanket1729)
d22e0149ad Taproot psbt impl BIP 371 (sanket1729)
108fc3d4db Impl encodable traits for TapLeafhash (sanket1729)
c7478d8fd0 Derive serde for taproot stuctures (sanket1729)

Pull request description:

  Built on top of #677 . Will rebase and mark ready for review after #677 is merged.

ACKs for top commit:
  apoelstra:
    ACK 7d982fa9a2
  dr-orlovsky:
    re-tACK 7d982fa9a2 basing on `git range-diff`. The original PR before last re-base was tested commit-by-commit.

Tree-SHA512: feb30e4b38d13110a9c0fabf6466d8f0fb7df09a82f4e01d70b8371b34ab0187004a6c63f9796c6585ee30841e8ee765ae9becae139d2e1e3d839553d64c3d1e
2021-12-30 02:12:03 +02:00
Dr. Maxim Orlovsky 86055d9df5
Merge rust-bitcoin/rust-bitcoin#672: New Witness struct to improve ser/de perfomance
106acdc3ac Add fuzzing for Witness struct (Riccardo Casatta)
2fd0125bfa Introduce Witness struct mainly to improve ser/de performance while keeping most usability. (Riccardo Casatta)

Pull request description:

  At the moment the Witness struct is  `Vec<Vec<u8>>`, the vec inside a vec cause a lot of allocations, specifically:

  - empty witness -> 1 allocation, while an empty vec doesn't allocate, the outer vec is not empty
  - witness with n elements -> n+1 allocations

  The proposed Witness struct contains the serialized format of the witness. This reduces the allocations to:

  - empty witness -> 0 allocations
  - witness with n elements -> 1 allocation for most common cases (you don't know how many bytes is long the entire witness beforehand, thus you need to estimate a good value, not too big to avoid wasting space and not too low to avoid vector reallocation, I used 128 since it covers about 80% of cases on mainnet)

  The inconvenience is having slightly less comfortable access to the witness, but the iterator is efficient (no allocations) and you can always collect the iteration to have a Vec of slices. If you collect the iteration you end up doing allocation anyway, but the rationale is that it is an operation you need to do rarely while ser/de is done much more often.

  I had to add a bigger block to better see the improvement (ae860247e191e2136d7c87382f78c96e0908d700), these are the results of the benches on my machine:

  ```
  RCasatta/master_with_block
  test blockdata::block::benches::bench_block_deserialize                 ... bench:   5,496,821 ns/iter (+/- 298,859)
  test blockdata::block::benches::bench_block_serialize                   ... bench:     437,389 ns/iter (+/- 31,576)
  test blockdata::block::benches::bench_block_serialize_logic             ... bench:     108,759 ns/iter (+/- 5,807)
  test blockdata::transaction::benches::bench_transaction_deserialize     ... bench:         670 ns/iter (+/- 49)
  test blockdata::transaction::benches::bench_transaction_get_size        ... bench:           7 ns/iter (+/- 0)
  test blockdata::transaction::benches::bench_transaction_serialize       ... bench:          51 ns/iter (+/- 5)
  test blockdata::transaction::benches::bench_transaction_serialize_logic ... bench:          13 ns/iter (+/- 0)

  branch witness_with_block (this one)
  test blockdata::block::benches::bench_block_deserialize                 ... bench:   4,302,788 ns/iter (+/- 424,806)
  test blockdata::block::benches::bench_block_serialize                   ... bench:     366,493 ns/iter (+/- 42,216)
  test blockdata::block::benches::bench_block_serialize_logic             ... bench:      84,646 ns/iter (+/- 7,366)
  test blockdata::transaction::benches::bench_transaction_deserialize     ... bench:         648 ns/iter (+/- 77)
  test blockdata::transaction::benches::bench_transaction_get_size        ... bench:           7 ns/iter (+/- 0)
  test blockdata::transaction::benches::bench_transaction_serialize       ... bench:          50 ns/iter (+/- 5)
  test blockdata::transaction::benches::bench_transaction_serialize_logic ... bench:          14 ns/iter (+/- 0)
  ```

  With an increased performance to deserialize a block of about 21% and to serialize a block of about 16% (seems even higher than expected, need to do more tests to confirm, I'll appreciate tests results from reviewers)

ACKs for top commit:
  apoelstra:
    ACK 106acdc3ac
  sanket1729:
    ACK 106acdc3ac
  dr-orlovsky:
    utACK 106acdc3ac

Tree-SHA512: e4f23bdd55075c7ea788bc55846fd9e30f9cb76d5847cb259bddbf72523857715b0d4dbac505be3dfb9d4b1bcae289384ab39885b4887e188f8f1c06caf4049a
2021-12-30 01:55:44 +02:00
Riccardo Casatta e511670e43
Merge rust-bitcoin/rust-bitcoin#722: Allow specifing a raw `TapLeafHash` in sighash computation
2959e04ebd Allow specifing a raw `TapLeafHash` in sighash computation (Alekos Filini)

Pull request description:

  Still need to add some tests but the code should be ready for review. Please let me know if you have better ideas for the enum naming.

  ---

  Instead of always requiring the full raw script and leaf version, allow
  just specifying a raw leaf hash to the sighash computation functions.

  This is very useful when dealing with PSBTs, because the
  `PSBT_IN_TAP_BIP32_DERIVATION` field only maps a public key to a leaf
  hash, so a signer could just take it and produce a signature with it
  rathern than having to jump through hoops to recover the full raw
  script.

ACKs for top commit:
  sanket1729:
    Tested locally. ACK 2959e04. Reviewed range-diff with 5aa1d02
  apoelstra:
    ACK 2959e04ebd

Tree-SHA512: 830be0b8382ac59b73e6481f61ec1effdcd32859c04382e6cd5a43ac689d6e528f9a8b27c026ee81f5d5b59d2e3c397f9c271145e001ff2dc4815764fc21a2c6
2021-12-29 18:08:45 +01:00
sanket1729 7d982fa9a2 Add all tests from BIP 371 2021-12-28 20:40:58 +05:30
sanket1729 d22e0149ad Taproot psbt impl BIP 371 2021-12-28 20:40:58 +05:30
sanket1729 108fc3d4db Impl encodable traits for TapLeafhash 2021-12-28 20:40:58 +05:30
sanket1729 c7478d8fd0 Derive serde for taproot stuctures 2021-12-28 20:40:58 +05:30
Riccardo Casatta 2fd0125bfa
Introduce Witness struct mainly to improve ser/de performance while keeping most usability.
Witness struct is in place of the Vec<Vec<u8>> we have before this commit.

from_vec() and to_vec() methods are provided to switch between this type and Vec<Vec<u8>>

Moreover, implementation of Default, Iterator and others allows to have similar behaviour but
using a single Vec prevent many allocations during deserialization which in turns results in
better performance, even 20% better perfomance on recent block.

last() and second_to_last() allows to access respective element without going through costly Vec
transformation
2021-12-28 09:56:38 +01:00
sanket1729 b945a5e5c6
Merge rust-bitcoin/rust-bitcoin#665: transactions: add a note about `get_vsize` and standardness rules
826fed53f2 transactions: add a note about `get_vsize` and standardness rules (Antoine Poinsot)

Pull request description:

  If they ever hit a discrepancy they must really be doing something dodgy but hey :)

ACKs for top commit:
  dr-orlovsky:
    ACK 826fed53f2

Tree-SHA512: c618a80b047797625a233939d2c1146e8b4ce44215648841813f78178577afc844f5e561e4e60b4084e315735894ecb354af8d81f4702f5354e5d5cd05b52ac4
2021-12-28 02:35:54 +05:30
Alekos Filini 2959e04ebd
Allow specifing a raw `TapLeafHash` in sighash computation
Instead of always requiring the full raw script and leaf version, allow
just specifying a raw leaf hash to the sighash computation functions.

This is very useful when dealing with PSBTs, because the
`PSBT_IN_TAP_BIP32_DERIVATION` field only maps a public key to a leaf
hash, so a signer could just take it and produce a signature with it
rathern than having to jump through hoops to recover the full raw
script.
2021-12-27 16:18:19 +01:00
Riccardo Casatta 9e1f256b54
Merge rust-bitcoin/rust-bitcoin#731: Improve parsing of `Denomination` string
f690b8e362 Be more liberal when parsing Denomination (Tobin Harding)
628168e493 Add missing white space character (Tobin Harding)

Pull request description:

  There is no reason to force users to use a particular format or case for `Denomination` strings. Users may wish to write any of the following and all seem reasonable
  - 100 sats
  - 100 sat
  - 100 SAT

  The same goes for various other `Denomination`s.

  - Patch 1 enables usage of "sats", "sat", "bit", "bits"
  - Patch 2 enables usage of various lower/uper case formatting

  Fixes: #729

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

Tree-SHA512: a785608e19a7ba6f689dc022cb17a709041ff56abeaa74649d0832a8bd8aac4593c7a79b46a47dd417796c588d669f50fb3c8b8a984be332ca38a1fef2dcd4ce
2021-12-27 10:17:40 +01:00
sanket1729 3eea63e42b Re-export SigHashType in lib.rs
Using the latest version of rust-bitcoin master on rust-miniscript
errors on bitcoin::SigHashType not found. In the original PR, I only
renamed the export to ECDSASigHashType, but original re-export should
also be there in lib.rs to avoid to breaking changes downstream.
2021-12-26 04:56:25 +05:30
Riccardo Casatta f9b3fc9ce8
Merge rust-bitcoin/rust-bitcoin#686: Fixed a bunch of clippy lints, added clippy.toml
779d4110c6 Fixed a bunch of clippy lints, added clippy.toml (Martin Habovstiak)

Pull request description:

  This is the initial step towards using and maybe enforcing clippy.
  It does not fix all lints as some are not applicable. They may be
  explicitly ignored later.

  Some discussion about clippy was in #685

ACKs for top commit:
  apoelstra:
    ACK 779d4110c6
  RCasatta:
    ACK 779d4110c6

Tree-SHA512: fb9192c77565a0b1b2118877c6413945d65900e4e95b3741107bf6cddef1fa65ff09fc5b7814de421382292321cca6bd860bf17b73a227d193a0a13758ee25eb
2021-12-24 09:41:03 +01:00
Riccardo Casatta 6fa8a82414
Merge rust-bitcoin/rust-bitcoin#695: BIP341 test vectors
7aacc3782a Add tests from BIP341 (sanket1729)
61629cc733 Make taproot hashes forward display (sanket1729)

Pull request description:

  Add tests for taproot.
  - ~Also fixes one bug in #677, namely, I was returning `LeafVersion::default()` instead of given version~
  - ~ Fixes a bug in #691 about taking secp context as a reference instead of consuming it. This should have not passed my review, but this is easy to miss. ~
  - Makes the display on taproot hashes forward instead of the reverse (because the BIP prints in a forward way, I think we should too and it is more natural. )

ACKs for top commit:
  RCasatta:
    ACK 7aacc3782a
  apoelstra:
    ACK 7aacc3782a

Tree-SHA512: 2e0442131fc036ffa10f88c91c8fc02d9b67ff6c16c592aa6f4e6a220c26a00fc6ca95a288f14aa40667a289fb0446219fd6c76c0196ead766252356592b9941
2021-12-23 15:32:49 +01:00