b02c7d1d33 Derive Copy for WitnessProgram (Tobin C. Harding)
Pull request description:
Recently we started using our custom `ArrayVec` for the `program` field of `WitnessProgram`, this means we can now derive `Copy`.
Fix: #2313
ACKs for top commit:
apoelstra:
ACK b02c7d1d33
sanket1729:
ACK b02c7d1d33
Tree-SHA512: 5741081d578f7b056c156d046dc3b0817b4b13cf69dcc1dfb8c7f4dbe8a4f9ed6c8802aaaf2b0084dbf3984d3fde807a02dbaa8c3bd29c220b3b32d3cb7c9f38
a8d50a5541 Remove Push enum (Tobin C. Harding)
Pull request description:
The `Push` enum is only ever used to get access to one of its variants. Since it is a private type we can remove it entirely and just return `PushBytes` from the `last_pushdata` function.
Needs careful review but I believe the function name is still correctly descriptive.
This was discovered by of a new nightly clippy warning.
ACKs for top commit:
apoelstra:
ACK a8d50a5541 Looks good to me. The latest compiler complains about the currently-unused variants.
sanket1729:
ACK a8d50a5541.
Tree-SHA512: 7f96057b0f6f5673252578253ad4f1789793dbf6e917d3974274dedf942da27e6247946262a0669eb500d47987788fcca0e020ed16c0d672188e95ee31163242
089ce8f0fb Deprecate `Script::is_provably_unspendable` (Martin Habovstiak)
Pull request description:
This method is not really that useful because it checked an arbitrary condition. There already exists `OP_RETURN` semantics and the method didn't cover all possible ways the script may be invalid.
This deprecates the method and documents why.
Closes#2191
ACKs for top commit:
apoelstra:
ACK 089ce8f0fb
tcharding:
ACK 089ce8f0fb
sanket1729:
ACK 089ce8f0fb
Tree-SHA512: 044f1c06fb8cbea4f84817be41bf10315f690b2a42748a07c1dd1eb0ba10932456780956fc628fec4bf57fe0722129537874a77be482d6660f9e02de5fc5a8a0
The `Push` enum is only ever used to get access to one of its variants.
Since it is a private type we can remove it entirely and just return
`PushBytes` from the `last_pushdata` function.
Needs careful review but I believe the function name is still correctly
descriptive.
03bfe1d433 Impove rustdoc on assume_checked_ref (Tobin C. Harding)
769809f1f2 Improve the docs on as_unchecked function (Tobin C. Harding)
Pull request description:
In #1765 we added a couple of new functions.
- Patch 1: Fix mis-documented function.
- Patch 2: Do trivial rustdocs fix.
ACKs for top commit:
apoelstra:
ACK 03bfe1d433
Kixunil:
ACK 03bfe1d433
Tree-SHA512: 5be6b2d288c1f4e9096014acd8618dc84a3ec6f45ae38b5d44ef7f95eccc268021bc8e8435152166606d893d4238b03e59e8f9d4fc67ba9a6c33194f3f54fc40
429a3ecec4 Add the implementation of `Display` for `transaction::Version` (harshit933)
Pull request description:
Adds the implementation of `Display` trait for `transaction::Version`
fixes#2308
This is unrelated to the issue but can anyone suggest some good issues that needs to be fixed. I am also taking a look but I am confused as to which I would be able to solve. I am here to learn more.
Thank you.
ACKs for top commit:
apoelstra:
ACK 429a3ecec4 Merry Christmas
tcharding:
ACK 429a3ecec4
Tree-SHA512: 9e59a8fe494b01caa8f211441744709f26df03891be171242bea4f7ccd7c3cc58b548cad241cab5270ad66fc9bb33ea7d6f98cc60d496c47647fb3396db9410f
The `as_unchecked` method is never dangerous to call because an
`Address<UncheckedNetwork>` provides a subset of functionality that is
always ok to use. It is only dangerous to go the other way unchecked to
checked.
This lint triggers on `fn input_len(&self) -> usize { match *self {} }`
where Self is an infallible type, claiming that the dereference of self
is UB. Maybe it would be, if this were possible. But it's not, and this
is literally the only point of using infallible types, so this lint is
always wrong.
Enabled in rustc 1.76 as warn by default.
This method is not really that useful because it checked an arbitrary
condition. There already exists `OP_RETURN` semantics and the method
didn't cover all possible ways the script may be invalid.
This deprecates the method and documents why.
8783d526bd fix : adds the arrayvec dependency (harshit933)
Pull request description:
This commit adds the arrayvec dependency to the sortKey.
Potential fix#2276
ACKs for top commit:
Kixunil:
ACK 8783d526bd
apoelstra:
ACK 8783d526bd
Tree-SHA512: 35f28ade02dd526ce5dfa2f42578b36cd5af29a5a9f409da70a775bc12046674737e9bce9fabcc87f1b4669080ad10465c75601342f280c11eab11f791f44c36
BIP-32 only differentiates between mainnet and some testnet when
encoding and decoding xpubs and xprivs. As such we can use the new
`NetworkKind` type instead of `Network` throughout the `bip32` module.
We only use the network to serialize and deserialize from WIF.
For this we only really need network kind since WIF only differentiates
between mainnet and non-mainnet.
Add a new type `NetworkKind` the describes the kind of network we are
on, ether mainnet or one of the test nets (testnet, regtest, signet).
Do not use the type yet.
a92d49fe33 Implement `CompressedPublicKey` (Martin Habovstiak)
Pull request description:
P2WPKH requires keys to be compressed which introduces error handling even in cases when it's statically known that a key is compressed. To avoid it, this change introduces `CompressedPublicKey` which is similar to `PublicKey` except it's statically known to be compressed.
This also changes relevant code to use `CompressedPublicKey` instead of `PublicKey`.
ACKs for top commit:
tcharding:
ACK a92d49fe33
apoelstra:
ACK a92d49fe33
Tree-SHA512: ff5ff8f0cf81035f042dd8fdd52a0801f0488aea56f3cdd840663abaf7ac1d25a0339cd8d1b00f1f92878c5bd55881bc1740424683cde0c28539b546f171ed4b
43b1ed1b86 Fully encapsulate bitcoinconsensus (Tobin C. Harding)
Pull request description:
The `bitcoinconsensus` crate is not fully under our control because it exposes code from Core, so we cannot guarantee its stability across versions. To make our semver compliance easier we can fully encapsulate the `bitcoinconsensus` crate so it does not appear in our public API.
### Please note that with this applied:
- The `bitcoinconsenus` crate is no longer exported at the crate root
- No `bitcoinconsensus` types appear in our public API
ACKs for top commit:
Kixunil:
ACK 43b1ed1b86
apoelstra:
ACK 43b1ed1b86
Tree-SHA512: 9fc4f01a35396562e980a647784b22667cbd289e45b5c122610d23a1f8bcf0fe8b9c27e33745f14ee010050d4c2d2669b679fb39c7a108e4e86d2c14fd60571a
The `bitcoinconsensus` crate is not fully under our control because it
exposes code from Core, so we cannot guarantee its stability across
versions. To make our semver compliance easier we can fully encapsulate
the `bitcoinconsensus` crate so it does not appear in our public API.
However, it is useful to have the crate itself exported, here we add an
"unstable" feature and only publicly export the `bitcoinconsensus` crate
if the "unstable" feature is enabled.
P2WPKH requires keys to be compressed which introduces error handling
even in cases when it's statically known that a key is compressed. To
avoid it, this change introduces `CompressedPublicKey` which is similar
to `PublicKey` except it's statically known to be compressed.
This also changes relevant code to use `CompressedPublicKey` instead of
`PublicKey`.
There is no advantage in having `io::Read` as opposed to `Read` and
importing the trait. It is surprising that we do so.
Remove `io::` path from `io::Read` and `io::Write`. Some docs keep the
path, leave them as is. Add import `use io::{Read, Write}`.
Refactor only, no logic changes.
When we use the `fmt::Write` trait it is just to call its methods, we
can therefore, without any change to the logic, use `as _` when
importing the trait. This prevents naming conflicts.
Done in preparation for importing the `io::Write` trait.
Generic types can be single letters, and a writer is conventionally, in
this codebase at least, called `W`.
Use `W` instead of `Write` with no loss of clarity.
1ee989a3af Remove private fmt_internal function (Tobin C. Harding)
923ce7402d Remove Network from AddressInner (Tobin C. Harding)
3490433618 Return error from wpubkey_hash (Tobin C. Harding)
f7ab253ce4 Remove stale comment (Tobin C. Harding)
Pull request description:
An `AddressInner` struct (contains `Network` field) is created when parsing address strings however address strings do not map 1:1 to `Network` because signet and testnet use the same bech32 prefix "tb".
We can fix this by inlining the `Payload` variants into `AddressInner` and adding prefix enums for legacy addresses and an `Hrp` for bech32 addresses.
Fix: #1819
ACKs for top commit:
Kixunil:
ACK 1ee989a3af
apoelstra:
ACK 1ee989a3af
Tree-SHA512: 1c2749dc929a1e9ad9b9feb01bec5c96b5aec07c6d646d88652deca7abe485907403116e9e29a0ab7dc06223254c4b49a384043284ec0a68fd76f9ab551e9e8a
396e049a7a Use InputString instead of String (Tobin C. Harding)
acacf45edf Add ParseDenominationError (Tobin C. Harding)
69e56a64ed Add bitcoin-units crate (Tobin C. Harding)
4ecb1fe7da internals: Add docs to InputString (Tobin C. Harding)
fa8d3002cd internals: Fix docs typo (Tobin C. Harding)
Pull request description:
Create a new `bitcoin-units` crate as described [here](https://github.com/rust-bitcoin/rust-bitcoin/issues/550#issuecomment-1012103022).
Only the `amount` module is currently included.
I've resolved the `Encodale/Decodable` issue by keeping the `amount` module in `bitcoin`.
ACKs for top commit:
Kixunil:
ACK 396e049a7a
apoelstra:
ACK 396e049a7a
Tree-SHA512: caf5e9da0458435ab19d00d4506896257e898525a4472d435fdac1d1a37bb747befd56993b106673f938475e5777d952a13ba04a2d3cb710d7afe7f5faebb7b8
e1cc98986c Put `#[inline]` on trivial functions (Martin Habovstiak)
e531fa612b Move `TaprootMerkleBranch` and impl `IntoIterator` (Martin Habovstiak)
9d23c1d0a8 Implement std traits for `TaprootMerkleBranch` (Martin Habovstiak)
93b415589d Rename `inner` to `slice`/`vec` (Martin Habovstiak)
bb0f839c2f Lint with nightly (Martin Habovstiak)
Pull request description:
This contains several improvements to `TaprootMerkleBranch` that make the API more idiomatic.
ACKs for top commit:
tcharding:
ACK e1cc98986c
apoelstra:
ACK e1cc98986c
Tree-SHA512: b2bf52b027e7c1f8588c54e8b8d7a5fa54011dc521bd917995011d5fcc16c50a486eb89c0cdae2557a58adbe7708a4f2bc8f4c492e3d88c679f2abf85b1e7c83
1b23220d10 Fix: TxOut::minimal_non_dust and Script::dust_value (Jonathan Underwood)
Pull request description:
Fixes#2192
TxOut::minimal_non_dust has 3 problems.
1. There is an invisible dependency on Bitcoin Core's default minrelaytxfee value. It has been made explicit.
2. There is an off by one error. The dust limit comparison uses < and therefore `+ 1` was not needed. It has been fixed.
3. It was not returning 0 amount for OP_RETURN outputs.
Script::dust_value has 2 problems.
1. The dust amount depends on minrelaytxfee which is configurable in Bitcoin Core. This method was not configurable.
2. The division operation was done before multiplying the byte amount, which can cause small differences when using uncommon scripts and minrelaytxfee values.
ACKs for top commit:
Kixunil:
ACK 1b23220d10
apoelstra:
ACK 1b23220d10
Tree-SHA512: eafd5112fbf773d86e094e3a69c519dd32f5074f5c9c63a8d69b1c9796579a8f2c2d11ad0995d8252c25b7fed5cd7c968ab88a70588986981a0a63649d43e197
c7c553ebc0 Remove impossible InvalidParity error variant (Martin Habovstiak)
Pull request description:
Since we do `& 1`, only 0 and 1 are possible values, so the error return there can never happen. I made this explicit by manually setting the parity.
This is a rebase of Steven's change #2163 with a rewrite of `match` to not panic.
ACKs for top commit:
tcharding:
ACK c7c553ebc0
apoelstra:
ACK c7c553ebc0
Tree-SHA512: 5591fda686295f330b6da757a0052687eddec135ac947a2801343f1681bc4fd9f6cfb722ac1339ae6187a8784e7629b5f12cca32b33a81ffc8791b4407b29f85
Tobin C. Harding
Andrew Poelstra
Fmt Bot
GoodDaisy
shuoer86
conduition
yancy
harshit933
Martin Habovstiak