95b7a95fc2 test: correct psbt used for regression test (Subhradeep Chakraborty)
Pull request description:
The `unknown` key used in the `Psbt` (in `serde_regression_psbt` test) is not really unknown. Also the `preimages` in the `Input` don't seem to be valid. Though they don't affect the tests because of the `serde::serialize`, the `Psbt` itself is invalid and hence this PR.
ACKs for top commit:
tcharding:
ACK 95b7a95fc2
apoelstra:
ACK 95b7a95fc2
Tree-SHA512: 275320bb273163364640351e55a3f16c34c1eef06e9dde79b72bf40187d313f037edb5e4544f4dce32fb8a7cc552504aa2e4d22a646c89a3dec022f917539219
b2a7d7023c Rename XpubIdentifier to XKeyIdentifier (Tobin C. Harding)
ffd2466ad1 Move XpubIdentifier to the bip32 module (Tobin C. Harding)
Pull request description:
- Patch 1: Move the hash to the `bip32` module where it is used, as we have done with other hashes recently (and re-export it at crate root).
- Patch 2: Rename the hash to `XKeyIdentifier` as discussed in #2014Fix: #2014
ACKs for top commit:
apoelstra:
ACK b2a7d7023c
Tree-SHA512: 5efa9fc857c71e506263bf6adee3b4294f22838d5b119177c9108c69191d545338c11a4796bc95e956a67f3418010725f3d12c06d2c4c3bb5cf038d59976ae0f
71a5fe2b54 Customize Debug implementation of absolute::LockTime (Subhradeep Chakraborty)
Pull request description:
Fixes https://github.com/rust-bitcoin/rust-bitcoin/issues/2011.
This PR aims to make the "pretty print" of `absolute::LockTime` prettier by printing `X blocks` and `X seconds` for `Blocks` and `Seconds` respectively instead of the default Enum printing.
ACKs for top commit:
apoelstra:
ACK 71a5fe2b54
tcharding:
ACK 71a5fe2b54
Tree-SHA512: 79baad5ee0ba1aa892cb38588dae6a24977b8e42356208d6cc9adb007d1f8371e61dc82e35bb4dac9961216d68c56907f0db376c5c6fbb5a406728b8f7c3f5ad
bc398204bf Remove redundant segwit version from function names (Tobin C. Harding)
Pull request description:
A P2TR output does not need to be clarified with version 1, it is implicit. As with p2wpkh/p2wsh and version 0.
Remove redundant version identifiers from function names, deprecating the originals.
ACKs for top commit:
apoelstra:
ACK bc398204bf
Tree-SHA512: 49806c564badca25ce02161445b2b41497b565f2002aa1edfc0cf0c57b38683480deec0d9b682e18dc7e59c22128e0b641abcccc2cbedd0b5603cbcbf2fd26df
f17bb0d18f Remove unnecessary reference (Tobin C. Harding)
Pull request description:
`T` is a generic that implements`AsRef<PushBytes>`, it should not be a reference. This is inline with other usages of `AsRef<PushBytes>` for example in `Builder::push_slice`.
Found while working on #2003
ACKs for top commit:
apoelstra:
ACK f17bb0d18f
Tree-SHA512: 6f6ae0ba5d5010db53d9c2af107df84bc058277b2b7cc35800f4e6ed93d351838b7f101284b7d80345bee639615d27d76a2e5c4c784782c5b3e5090444defe29
52f2332383 Remove docs from witness version conversion functions (Tobin C. Harding)
47d6d785cb Remove bip 173/350 test vectors (Tobin C. Harding)
e0eaeaad99 Split ParseError out of Error (Tobin C. Harding)
0f536e86dc Add new UnknownAddressTypeError for parsing address type (Tobin C. Harding)
e2014cba1b Import error variants within dislay impl (Tobin C. Harding)
9d7791fcd6 Remove unnecessary self:: from error import (Tobin C. Harding)
b2e485ed51 Split the address error code out into a separate module (Tobin C. Harding)
f34ca0c52b Move address.rs to address/mod.rs (Tobin C. Harding)
Pull request description:
In preparation for depending on the recently released version of `rust-bech32` do a bunch of preparatory fixes.
1. Improve `address` module error handling as we are doing else where at the moment
2. Remove bip 173 and 350 test vector tests, these are fully covered in bech32
3. Trim down the docs on `WitnessVersion`
This PR is the first 8 patches of https://github.com/rust-bitcoin/rust-bitcoin/pull/1951
ACKs for top commit:
sanket1729:
ACK 52f2332383
apoelstra:
ACK 52f2332383
Tree-SHA512: 67a4ea4020b4e5c9c8396e4195e06dbd1d11335788f9e52f60abbc0b399e37e5dacc9bb7fa4e88221670322fa3c3407ade059d5c709f96e2df97240f4524e08c
ccdcffe69c Re-export Opcode (Tobin C. Harding)
Pull request description:
We recently rename `opcodes::All` to `Opcode` but did not re-export it from the crate root. Since it now has a nice clear name we can do so.
Found while hacking up the `rust-bitcoincore-rpc` dependency upgrade for upcoming release `bitcoin v0.31.0`.
ACKs for top commit:
apoelstra:
ACK ccdcffe69c
Tree-SHA512: 8acac90d01f14245f93edc10f3483d8aa9865aca59b6ef42ab744b875558fca8ad74894ad95a83637f0cec7a2a353d74d4d6f7ee5f1d1276cc0fc3dcb6983362
acbf23aaa5 Add `is_multisig` helper to Script type (Clark Moody)
Pull request description:
A new `is_multisig` helper method to classify bare multisig output scripts.
The form of a valid multisig script is:
- Pushnum `M`
- <N> pubkeys
- Pushnum `N`
- `OP_CHECKMULTISIG`
`N` must equal the number of pushed pubkeys, and `M` must be less than or equal to `N`.
I've tested this against the RPC output of Core at the block level, checking that the total number of multisig outputs matches.
```
Block 350338, 89 multisig
Block 350340, 29 multisig
Block 350341, 4 multisig
Block 350343, 579 multisig
Block 350344, 48 multisig
Block 350346, 11 multisig
Block 350347, 404 multisig
Block 350350, 127 multisig
Block 350351, 1 multisig
Block 350353, 40 multisig
Block 350356, 13 multisig
Block 350357, 2 multisig
Block 350358, 1 multisig
```
ACKs for top commit:
tcharding:
ACK acbf23aaa5
apoelstra:
ACK acbf23aaa5
Tree-SHA512: b8feeaa8725ac63a658897dac3b303fc8b3d56674d796b14569548124928329993bea45482928d9ce85231f1b5837922af8c0a77b2601a92f88b5e2a9394e97f
026a55809e Fix: Script::count_sigops parsing should not return a Result (junderw)
Pull request description:
When implementing some tests for the Transaction PR, I noticed that there were coinbase transactions that would pass Bitcoin Core parsing and fail my code.
It turns out that the Script parsing for sigops calls `break` to exit the loop and returns the current n value whenever there is an EarlyEndOfScript error.
See this comment: https://github.com/rust-bitcoin/rust-bitcoin/pull/2073#issuecomment-1722403687 for some links to the relevant source.
ACKs for top commit:
apoelstra:
ACK 026a55809e
tcharding:
ACK 026a55809e
Tree-SHA512: 57c1b88add5e1c9ef9245fcec0e471db55c2f9b1b0b0f8ebd471f1bede0ca5eeb8492d8c75dea1fd43f1343037df44969c9b9fde26a7de9ac68a26dca899e47f
These docs do not add that much value, we do not typically bother
documenting `From` and `TryFrom` implementations because they are super
well known and its obvious from the function signature what is going on.
The `address::Error` is module level general, we can make the code
easier to maintain and easier to stabalize by splitting the parse error
out of the general error.
Create a `ParseError` that is returned by `FromStr for Address`. Remove
the now unused variants from the general `address::Error`.
In an effort to reduce the number of lines of code import the error
variants locally within the `Display` impl on `Error`.
Refactor only, no logic changes.
Split the error code out of `address/mod.rs` and into
`address/error.rs`. Code move only, no changes other than to
imports/exports etc. to make it build.
a0a3d4728a Fix deprecation notice (Tobin C. Harding)
Pull request description:
Recently we deprecated the `segwit_signature_hash` function but during development the deprecation notice got stale.
Fix deprecation notice to use the actual function names.
ACKs for top commit:
RCasatta:
ACK a0a3d4728a
apoelstra:
ACK a0a3d4728a
Tree-SHA512: d84941b605c5bc6ceab75cd60eb820c1d2c16fcd1431dc3927dc22d79886d3de26fd796fab92d97e7f8d567eab0b5a1987303107720524e7b648b1168541a2ed
7309c7749a Split witness version errors up (Tobin C. Harding)
40db2f5ed6 witness_version: Remove rustdocs from TryFrom imlps (Tobin C. Harding)
3397ff9910 witness_version: Use Self in error From impl (Tobin C. Harding)
Pull request description:
Done as part of the push to have small specific errors instead of large general ones.
Split the `witness_version::Error` up into small specific errors.
The first two patches are preparatory clean up.
ACKs for top commit:
stevenroose:
ACK 7309c7749a
apoelstra:
ACK 7309c7749a
Tree-SHA512: 9e8b4bc5db3435c88aa6de9d92f668146b20b292c3609e2d4415ff0c32a0e3923bbe765333e0d7c255c326d65680015fc9cdf3e4a994727f4d0273dc396314df
4f43965ade Make Encodable/Decodable usage uniform (Tobin C. Harding)
Pull request description:
One encodes to a writer and decodes from a reader, most of the time in the consensus `Encodable`/`Decodable` traits we use generic `R`/`W` and variable `r`/`w` but there are other places that use other characters.
While touching these lines note also that there are a bunch of unneeded `mut`s, I'm not sure why since usually between the compiler and the linter `mut` is handled correctly.
Make implementations of `Encodable` and `Decodable` uniform by:
- Use R/W and r/w for trait and variable name
- Remove unneeded mut
(This is split out of #1891 to assist review.)
ACKs for top commit:
apoelstra:
ACK 4f43965ade
stevenroose:
ACK 4f43965ade
Tree-SHA512: 256d080b32e60a7cabf6db4945a18d7ff5b296cb848712238e33f9b1ff3cabbe7d76723fed61a04e4b2a6c9423f12c32ec10e3ebb633761122add50e6a6cc7c9
a68c42e113 Remove test from Transaction test names (yancy)
f796d6fef9 Use Weight type for scaled_size (yancy)
e746341f33 Add tests for scaled_size (yancy)
97b7a2dee9 Use Weight type for block base_size (yancy)
9536a9947c Add base_size test (yancy)
Pull request description:
Use Weight type for `base_size` in Transaction. Also a small re-factor to remove `test_` and `_tests` from the testname for transaction tests.
ACKs for top commit:
apoelstra:
ACK a68c42e113
tcharding:
ACK a68c42e113
Tree-SHA512: f4ab54143cbd9b1439912390f1e0857069a32b715477a4bc08692c5e32860a7090c95a92f78b118b17c1295c45a3bbdd209ba1d68c3a934341269235040e6911
de95bf52cb Use checked_sub (Tobin C. Harding)
Pull request description:
Recently we "if" guarded subtraction manually using `> 0`, we can better convey the meaning by using `checked_sub` and pattern match on the option.
Refactor only, no logic changes.
ACKs for top commit:
RCasatta:
utACK de95bf52cb
apoelstra:
ACK de95bf52cb
Tree-SHA512: 2514cc2d8af89158e5e5e5a866f3fadb4927ba07dfb4e077fd16a98acf638588bee5ce03e2dc73fbda0b5064c30d8773d3be583c03c2a5336b8738c212a9776f
As we have recently been doing, move the declaration of the hash type to
where it is used.
Move the `XpubIdentifier` hash declaration to the `bip32` module.
This is an API breaking change.
A P2TR output does not need to be clarified with version 1, it is
implicit. As with p2wpkh/p2wsh and version 0.
Remove redundant version identifiers from function names, deprecating
the originals.
`T` is a generic that implements`AsRef<PushBytes>`, it should not be a
reference. This is inline with other usages of `AsRef<PushBytes>` for
example in `Builder::push_slice`.
One encodes to a writer and decodes from a reader, most of the time in
the consensus `Encodable`/`Decodable` traits we use generic `R`/`W` and
variable `r`/`w` but there are other places that use other characters.
While touching these lines note also that there are a bunch of unneeded
`mut`s, I'm not sure why since usually between the compiler and the
linter `mut` is handled correctly.
Make implementations of `Encodable` and `Decodable` uniform by:
- Use R/W and r/w for trait and variable name
- Remove unneeded mut
Done as part of the push to have small specific errors instead of large
general ones.
Split the `witness_version::Error` up into small specific errors.
Recently we "if" guarded subtraction manually using `> 0`, we can better
convey the meaning by using `checked_sub` and pattern match on the
option.
Refactor only, no logic changes.
Recently we deprecated the `segwit_signature_hash` function but during
development the deprecation notice got stale.
Fix deprecation notice to use the actual function names.
84614d9997 Unit test debug print of witness with empty instruction (Tobin C. Harding)
e96be5ee6e Fix Witness debug display bug (Tobin C. Harding)
Pull request description:
When we introduce a custom `Debug` implementation for the `Witness` we introduced a bug that causes code to panic if the witness contains an empty instruction.
The bug can be verified by putting patch 2 first or by running `cargo run --example sighash` on master.
ACKs for top commit:
apoelstra:
ACK 84614d9997
RCasatta:
ACK 84614d9997
Tree-SHA512: d51891206ab15f74dda07eb29ff3f6c69dc3f983a5a5abb55685688548481a19f7c1d33aa1183a89c553ff2bc86cf41057c2bae33d75e8a7f3b801056775bf9e
55e94b5dea Remove test from test names for Weight type (yancy)
142dde64c3 Use Weight type for stripped_size (yancy)
cb76f3ec43 Add scale_by_witness_factor to Weight type (yancy)
38c9e9947e Add witness scale factor to the Weight type (yancy)
77552987ab Add from_wu_usize to Weight type (yancy)
1a88c887f5 Rename strippedsize to stripped_size (yancy)
3369257c75 Fix grammar (yancy)
Pull request description:
Return Weight type for the strippedize function.
ACKs for top commit:
apoelstra:
ACK 55e94b5dea
tcharding:
ACK 55e94b5dea
Tree-SHA512: ad3e4bc29380f22e20a6302c1b24c201c772be759c655c62ba4717840a01fcaa36f0f8442c9a3ba71c6400d6af47a9a815e6d90877b5f14c6883fb950b9669fd
29a4f9b114 Wrap the bitcoinconsensus error (Tobin C. Harding)
Pull request description:
Currently the `bitcoinconsensus` error is part of the public API. This hinders maintainability because changes to the verison of `bitcoinconsensus` force a re-release in `rust-bitcoin`. This is an unnecessary maintenance burden, we can wrap the error instead.
ACKs for top commit:
apoelstra:
ACK 29a4f9b114
sanket1729:
utACK 29a4f9b114
Tree-SHA512: 36bc1b0ad5f5675d79eea2409844a839d862997c256e301c53c5f1af547edc9a0b83e586bd70e1b8853722cd7ef279e7515e09fbe942660f8049090d1be39d3a
f18f684ad2 Add version bytes consts (Tobin C. Harding)
Pull request description:
BIP-32 defines 4 4-byte consts used as version bytes; currently we are hardcoding the version bytes in multiple places.
Add BIP-32 version bytes consts and use them throughout the module.
ACKs for top commit:
apoelstra:
ACK f18f684ad2
RCasatta:
utACK f18f684ad2
Tree-SHA512: 50bf2d26f0f8e3528642ffcc621c03b82f536994deb808a6c84225676b4b8849db8e0d16e46f3819e0810296a422b31cf90d0595739910afdb92fb768ef7696e
junderw
Andrew Poelstra
Subhradeep Chakraborty
Tobin C. Harding
Clark Moody
Riccardo Casatta
yancy