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
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
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.
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
66d5800ac0 psbt: Add IndexOutOfBounds error (Tobin C. Harding)
Pull request description:
We currently have a bunch of functions that are infallible if the `index` argument is within-bounds however we return a `SignError`, this obfuscates the code.
Add an `IndexOutOfBoundsError`. While we are at it make it an enum so users can differentiate between which vector the out of bounds access was attempted against.
ACKs for top commit:
sanket1729:
utACK 66d5800ac0. This is a clean improvement over existing code.
apoelstra:
ACK 66d5800ac0
Tree-SHA512: fa8a24990d1dcdab0c9b019fb2387b5a518b02d0a65715f0ab62519894b19c0c74750d3dcdc928626fa68b146038b907d79de3ba9712c9287db8fa64693ebc11
be05f9d852 Rename xpub and xpriv types (Tobin C. Harding)
Pull request description:
The BIP-32 extended public key and extended private key exist in the Bitcoin vernacular as xpub and xpriv. We can use these terms with no loss of clarity.
Rename our current BIP-32 types
- `ExtendedPubKey` to `Xpub`
- `ExtendedPrivKey` to `Xpriv`
This patch is a mechanical search-and-replace, followed by running the formatter, no other manual changes.
ACKs for top commit:
apoelstra:
ACK be05f9d852
sanket1729:
ACK be05f9d852
Tree-SHA512: 49925688783c3f37a9b92a9767a0df095323a3fa51f3d672a0b5dd1d8bca86f7facbcc33921274bc147b369de09042c4850b08c31e63f71110903435daa6c00c
724be17394 Remove useless usage of vec! macro (Tobin C. Harding)
e84ca292d9 Clear incorrect implementation of clone warning (Tobin C. Harding)
Pull request description:
A new version `clippy 0.1.72 (5680fa1 2023-08-23)` just came out and we get a two new warnings. Fix them up.
ACKs for top commit:
apoelstra:
ACK 724be17394
Tree-SHA512: f82f026773f8738d8d89710b36b979850e0e33c4d1afb4f78d2d4e957b37dd850ef9e83c9e25197dac981c7b49c448e49078777261749567303f1aac282b3d33
7bbdd9b2af Export all hash types (Tobin C. Harding)
Pull request description:
During the 0.30.0 release we removed the re-exports of hash types. This upset some folk and since the aim of our re-exports is not exactly clean as well as the fact that the public API surface is not yet fixed just re-export all the hash types at the crate root again.
This is #1792 but does not use a wildcard and also grabs the other hashes that we recently moved.
ACKs for top commit:
apoelstra:
ACK 7bbdd9b2af
sanket1729:
ACK 7bbdd9b2af
Tree-SHA512: addfb617fae2fce12eb9d198ffeb1b0c99792dfd757222c21c62bd4b408432de5dc93c42da7d886b43c29b2c34bd318573e813f567a29080fc264ee7beba0f70
53f68383b7 hashes: Bump version to 0.13.0 (Tobin C. Harding)
Pull request description:
In preparation for `hashes` release, bump the version. Depend on new version in `rust-bitcoin`.
Note the `bitcoin-private` addition to the lock files is because we temporarily have two `bitcoin_hashes` dependencies and the secp one (v.0.12.0) depends on `bitcoin-private`.
ACKs for top commit:
sanket1729:
utACK 53f68383b7 . I am not sure about the exact nature of release dance between various crates in rust-bitcoin. This code and changelog entries looks good.
apoelstra:
ACK 53f68383b7
Tree-SHA512: a1933bcda1fa9a06c96a4c7079ff49f531e4f366373e98700446cecdf1eed5a104d4d4aa737202ec426cb1f1edf88eff5eee80df9f0cc3a9779c051a55f847b5
We currently have a bunch of functions that are infallible if the
`index` argument is within-bounds however we return a `SignError`, this
obfuscates the code.
Add an `IndexOutOfBoundsErorr`. While we are at it make it an enum so
users can differentiate between which vector the out of bounds access
was attempted against.
Throughout the codebase we cast values to `u64` when constructing a
`VarInt`. We can make the code marginally cleaner by adding `From<T>`
impls for all unsigned integer types less than or equal to 64 bits.
Also allows us to (possibly unnecessarily) comment the cast in a single
place.
The `ThirtyTwoByteHash` trait is defined in `secp256k1` and used in
`hashes` as well as `bitcoin`. This means that we must use the same
version of `hashes` in both `bitcoin` and `secp256k1`. This makes doing
release difficult.
Remove usage of `ThirtyTwoByteHash` and use `Message::from_slice`.
Include TODO above each usage because as soon as we release the new
version of secp we can use the new `Message::from_digest`.
This is step backwards as far as type safety goes and it makes the code
more ugly as well because it uses `expect` but thems the breaks.
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.
The BIP-32 extended public key and extended private key exist in the
Bitcoin vernacular as xpub and xpriv. We can use these terms with no
loss of clarity.
Rename our current BIP-32 types
- `ExtendedPubKey` to `Xpub`
- `ExtendedPrivKey` to `Xpriv`
This patch is a mechanical search-and-replace, followed by running the
formatter, no other manual changes.
4300cf2210 Add p2wpkh and p2wsh signature hash functions (Tobin C. Harding)
Pull request description:
The word "segwit" refers to segwit v0 and taproot but these functions are version specific. Add `v0` into the function names.
This is similar to #1994, both based on recent post of mine to bitcoin dev mailing list.
ACKs for top commit:
stevenroose:
ACK 4300cf2210
apoelstra:
ACK 4300cf2210
Tree-SHA512: 723fc302954514da0fa57a3890b9f62e9d8d1b25289b8db00611d8bc34c5000b9e54943f57b8e94befcaf72633ac078b2ff66a1da0c5bb483cfaa584e3cb6014
7ec33d29eb refactor: developer doc first (yancy)
5496feb5c1 Add base weight const to TxIn (yancy)
Pull request description:
Add a base weight const to TxIn. I also used this const in strippedsize() and scaledsize(). As a different PR, I think strippedsize and scaledsize could return Weight instead of usize. Also added a small commit to re-arrange commit messages.
ACKs for top commit:
apoelstra:
ACK 7ec33d29eb
tcharding:
ACK 7ec33d29eb
Tree-SHA512: b20f95605ed664b88df0a5a178d48f15f27d90eb404c9707aef010c4504d7ffd4a3565c217710b9289f87ed2a0724fd8f7cc78a79a58547fe3ee87339c0d74c1
Currently if the witness has zero elements or any of the individual
witnesses is empty we panic. Panic is caused by subtracting 1 from a
zero length.
Check the length is non-zero before subtracting 1, print `[]` if empty.
During the 0.30.0 release we removed the re-exports of hash types. This
upset some folk and since the aim of our re-exports is not exactly clean
as well as the fact that the public API surface is not yet fixed just
re-export all the hash types at the crate root again.
This is 1792 but does not use a wildcard and also grabs the other
hashes that we recently moved.
The word "segwit" refers to segwit v0 and taproot but currently we have
`segwit_signature_hash` that is version specific (segwit v0).
- Rename `segwit_encode_signing_data_to` to
`segwit_v0_encode_signing_data_to`
- Add `p2wpkh_signature_hash` and `p2wsh_signature_hash` functions
We keep the single encode function because the error handling is better
that way.
While we are at it test the bip-143 test vectors against all the
sighash types of wrapped p2wsh.
fa10668a35 Eliminate a heap allocation from PartialMerkleTree encoding & decoding (Steven Roose)
Pull request description:
Just came across this and felt like doing this.
ACKs for top commit:
apoelstra:
ACK fa10668a35
tcharding:
ACK fa10668a35
Tree-SHA512: 7167c63077851c4c461b33292948d9b09fe21eb45d52ed278ecf884ce15bc8b21c14040fa1eb5f50bfe51c5cde10abc133d0c59be502de139408c0d107ffa7eb
63d0fa0164 Rename All to Opcode (Tobin C. Harding)
881d1b1a9d Move opcode aliases to the all module (Tobin C. Harding)
f3e8dbc392 Remove floating code comment (Tobin C. Harding)
dfd9384d14 opcodes: Fix whitespace (Tobin C. Harding)
Pull request description:
I may be missing something about the `All` type but it seems it can be re-named to `Opcode` with no loss of clarity.
The first few patches do some other clean ups, the last patch is the meat and potatoes.
ACKs for top commit:
apoelstra:
ACK 63d0fa0164
Tree-SHA512: c1b8be9e0c090d015c2bbfcb7ed6abe998ccada5a1eb0f6966c2a7dc462f8bbc45e6b99bf1387c736d2fca21a993a2c7fd5844c18b97a2e37ec43da517566ab1
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.
Andrew Poelstra
yancy
junderw
Tobin C. Harding
Riccardo Casatta
sanket1729
Steven Roose