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
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
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.
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`.
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.
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
ebdeed086e Cleanup imports (sanket1729)
382c8f9e4f Introduce PsbtSigHashType (sanket1729)
Pull request description:
We cannot really use `Psbt` for taproot because the sighash type is currently EcdsaSigHashType. We could introduce an enum with two options but then deser is not really clear, so I chose the approach in the current PR. Feedback or other ways to do this welcome :)
This is NOT related to #776
ACKs for top commit:
apoelstra:
ACK ebdeed086e
dr-orlovsky:
ACK ebdeed086e
Tree-SHA512: f9424cf3db09098d73f0d431a45ff86a47f11f7d40785bf95e58991fd4d16f0db0a9a3a63f898628b29c95bbd2ca901312a6a44ac6d8aec73a6a34710f6354a2
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.
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
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
eb09019720 Rename inner key field in PrivateKey and PublicKey (Dr Maxim Orlovsky)
Pull request description:
Since we already broke all possible key-related APIs with this release, I think this one is good to have with 0.28.
Closes#532
ACKs for top commit:
sanket1729:
utACK eb09019720
Kixunil:
ACK eb09019720
Tree-SHA512: 26cee647c403416f9ef6eaf208c08c9ea7162f2e04ee0210749349112bc5bf6b3cb83702a960ccded0b70e50f310c058de8ee12fa7eb30b4546469ad40603d6e
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
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.
Dr Maxim Orlovsky
sanket1729
Tobin Harding
KaFai Choi
Andrew Poelstra
Nils Loewen