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.
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`.
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.
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.
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.
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
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
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
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
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
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`.
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
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`.
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.
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
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
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
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
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.
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
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
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.
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
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.
Vector initialisation uses neither "Block" nor "Visual" stlye, this is
irregular for no added benefit.
Elect to use "Block" style (as defined by `rustfmt`).
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.
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).
We have a few instances of strange indentation:
- Incorrect number of characters
- Usage of neither "Block" style or "View" style (elect to use "Block")
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.
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.
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.
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
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
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
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
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
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
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
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
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
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
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
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 :)
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.
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`.
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
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.
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.
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
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
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
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.
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
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]
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.
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
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
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
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
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
sanket1729
Tobin Harding
Dr. Maxim Orlovsky
Andrew Poelstra
Jeremy Rubin
Andrew Ahlers
Rishabh Singhal
Noah Lanson
Riccardo Casatta
Casey Rodarmor