518f0970c9 Implement ArbitaryOrd for absolute::LockTime (Tobin C. Harding)
Pull request description:
At times we would like to provide types that do not implement `PartialOrd` and `Ord` because it does not make sense. I.e we do not want users writing `a < b`. This could range from kind-of-iffy to down-right-buggy (like comparing absolute locktimes).
However this decision effects downstream users who may not care about what the ordering means they just need to use it for some other reason e.g., to use as part of a key for a `BTreeMap` (as we do in `miniscript` requiring the `AbsLockTime` type).
A solution to this problem is to provide a wrapper data type that adds `PartialOrd` and `Ord` implementations. I wrote the `ordered` crate is for this very purpose.
ACKs for top commit:
apoelstra:
ACK 518f0970c9
Kixunil:
ACK 518f0970c9
Tree-SHA512: 05c753e650b6e2f181caf7dc363c4f8ec89237b42883bd695a64da0661436c9a7e715347f8fcf4fb19ce069cbf75a93032052e946f05fd8029f61860cf9c6225
271b45299f Improve Signature field names (Tobin C. Harding)
Pull request description:
Applies to both `ecdsa::Signature` and `taproot::Signature`.
Re-name the `Signature` fields with more descriptive names. The names used were decided upon in the issue discussion.
Impove rustdocs while we are at it.
Note, the change to `sign-tx-segwit-v0` is refactor only, the diff does not show it but we have a local variable already called `sighash_type` that is equal to `EcdsaSighashType::All`.
Fix: #2139
ACKs for top commit:
Kixunil:
ACK 271b45299f
apoelstra:
ACK 271b45299f
Tree-SHA512: b27221e922206b2510b073c0f38678f97f0c946707e3e679eee181faa170348101198706f9ca5803a55c799b0b330cc263cc103cd9beefff4e5c58d8fea77b8d
Applies to both `ecdsa::Signature` and `taproot::Signature`.
Re-name the `Signature` fields with more descriptive names. The
names used were decided upon in the issue discussion.
Impove rustdocs while we are at it.
Note, the change to `sign-tx-segwit-v0` is refactor only, the diff does
not show it but we have a local variable already called `sighash_type`
that is equal to `EcdsaSighashType::All`.
Includes a function argument rename as well, just to be uniform.
Fix: #2139
e762c53725 fix nightly clippy issues (Andrew Poelstra)
Pull request description:
I don't mind making these PRs, but I think we should start pinning our nightlies, and using a script to update the pin every week (or day, or whatever). When our CI breaks due to tooling changes, it means that our release branches will have permanently broken CI jobs, which I'd like to avoid in the future.
If the automatic pin update failed, we'd close it and manually create one that fixed the nw issues.
ACKs for top commit:
tcharding:
ACK e762c53725
Tree-SHA512: c61af0d69746258268b28be2ef7bd703e121c02fcf48c0625019e51407275b2b501aa31b2e65a548f8aaa903c49eda2d426c53e9530ba79b9bf1d82a690f4372
3a562db39d docs: Remove pinning from hashes readme (Tobin C. Harding)
Pull request description:
We do not need to pin with the current MSRV, remove the stale mention of it from the `hashes` readme file.
ACKs for top commit:
apoelstra:
ACK 3a562db39d will merge with one-ACK carveout
Tree-SHA512: a3f2017ab40e28d1e047ec5466e4b101dfc13280226c71ec5b36d46f54bed359eccbce16d811cee100fb134f3091f7c986c7b2afa3adaa15942bee9e08e730f1
08d2b203a5 Remove rustdoc about attribute (Tobin C. Harding)
eea0b697bf Fix stale docs (Tobin C. Harding)
Pull request description:
Do two miner docs fixes.
- Patch 1: Fixes some stale docs I left in when refactoring the `AddressInner` type.
- Patch 2: Uses code comments instead of rustdoc and fixes the issue linked below.
Fix: #2315
ACKs for top commit:
sanket1729:
utACK 08d2b203a5
Tree-SHA512: e68276473b451c02e230364d386d5b4b610c8ec73ff30e97e111cd339cc2238b65e08c9a83aac5f9d5d6c2e1dd1ee10fec727dfec3bca3cd7317b7049d689f4a
At times we would like to provide types that do not implement
`PartialOrd` and `Ord` because it does not make sense. I.e., we do not
want users writing `a < b`. This could range from kind-of-iffy to
down-right-buggy (like comparing absolute locktimes).
However this decision effects downstream users who may not care about
what the ordering means they just need to use it for some other reason
e.g., to use as part of a key for a `BTreeMap` (as we do in `miniscript`
requiring the `AbsLockTime` type).
A solution to this problem is to provide a wrapper data type that adds
`PartialOrd` and `Ord` implementations. I wrote the `ordered` crate is
for this very purpose.
Feature gate a new dependency on `ordered` and implement `ArbitraryOrd`
for `absolute::LockTime`.
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
774b405ba9 2024-01-07 automated rustfmt nightly (Fmt Bot)
Pull request description:
Automated nightly `rustfmt` changes by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
Kixunil:
ACK 774b405ba9
sanket1729:
ACK 774b405ba9. I was missing for 3 months and we have cool new stuff
Tree-SHA512: ff905c8971bc92c0716c969222d5425715773803ea05336e1bbae35c4e5cfb5cf975451452ebb769296fa84eabbbfa03672bbc0aa161f871edeefaaeb594c2ca
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
01df1417c7 use arrayvec to represent witness programs (conduition)
Pull request description:
Fixes https://github.com/rust-bitcoin/rust-bitcoin/issues/2261
Introduces a new constructor, `WitnessProgram::from_bytes`, which creates a witness program by copying the program content from a byte slice.
ACKs for top commit:
Kixunil:
ACK 01df1417c7
apoelstra:
ACK 01df1417c7
Tree-SHA512: 73b8f2785674cd99c3f5dfe0e2180ed256942a0c29bcb1d357e0bd84fddee5e62f3f230c6cd55a37322bc3a6011467e9b7dcf24d903b20f35c095a1a1f9a29ce
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
278229def5 Add allow for out of bounds indexing (yancy)
Pull request description:
Out of bounds indexing is a workaround for const panic until MSRV +1.57
ACKs for top commit:
apoelstra:
ACK 278229def5 Happy New Year! Will merge based on CI one-ack carveout.
Tree-SHA512: 5d525b682c28407e910ae45e8999fe6c95226d5079db917ccda296c68d4ed7a204c9ff1c5ea36d0ae647ee605780939028bf04a3948b1034c71e88cf6f03c782
504f77adca ci: revert #2301 "update download-artifact to v4" (Andrew Poelstra)
5fd731f095 Don't match on complex expression (Martin Habovstiak)
6cdbb04820 clippy: whitelist uninhabited_references lint (Andrew Poelstra)
Pull request description:
Clippy has introduced a couple new lints that are causing our CI to fail. `allow` them.
ACKs for top commit:
tcharding:
ACK 504f77adca
Kixunil:
ACK 504f77adca
Tree-SHA512: 4b2312dcd1645792fa7c08ca02e8ec9f6a13fc5c275559e78293bb55be93997e21f952cf1ac66c1291c4699b6c2976289222201d7e9bbf68349030d83804f8f7
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.
7e2321de75 Bump actions/download-artifact from 3 to 4 (dependabot[bot])
Pull request description:
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 3 to 4.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a href="https://github.com/actions/download-artifact/releases">actions/download-artifact's releases</a>.</em></p>
<blockquote>
<h2>v4.0.0</h2>
<h2>What's Changed</h2>
<p>The release of upload-artifact@v4 and download-artifact@v4 are major changes to the backend architecture of Artifacts. They have numerous performance and behavioral improvements.</p>
<p>For more information, see the <a href="https://github.com/actions/toolkit/tree/main/packages/artifact"><code>@actions/artifact</code></a> documentation.</p>
<h2>New Contributors</h2>
<ul>
<li><a href="https://github.com/bflad"><code>@bflad</code></a> made their first contribution in <a href="https://redirect.github.com/actions/download-artifact/pull/194">actions/download-artifact#194</a></li>
</ul>
<p><strong>Full Changelog</strong>: <a href="https://github.com/actions/download-artifact/compare/v3...v4.0.0">https://github.com/actions/download-artifact/compare/v3...v4.0.0</a></p>
<h2>v3.0.2</h2>
<ul>
<li>Bump <code>@actions/artifact</code> to v1.1.1 - <a href="https://redirect.github.com/actions/download-artifact/pull/195">actions/download-artifact#195</a></li>
<li>Fixed a bug in Node16 where if an HTTP download finished too quickly (<1ms, e.g. when it's mocked) we attempt to delete a temp file that has not been created yet <a href="hhttps://redirect.github.com/actions/toolkit/pull/1278">actions/toolkit#1278</a></li>
</ul>
<h2>v3.0.1</h2>
<ul>
<li><a href="https://redirect.github.com/actions/download-artifact/pull/178">Bump <code>@actions/core</code> to 1.10.0</a></li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="7a1cd3216c"><code>7a1cd32</code></a> Merge pull request <a href="https://redirect.github.com/actions/download-artifact/issues/246">#246</a> from actions/v4-beta</li>
<li><a href="8f32874a49"><code>8f32874</code></a> licensed cache</li>
<li><a href="b5ff8444b1"><code>b5ff844</code></a> Merge pull request <a href="https://redirect.github.com/actions/download-artifact/issues/245">#245</a> from actions/robherley/v4-documentation</li>
<li><a href="f07a0f73f5"><code>f07a0f7</code></a> Update README.md</li>
<li><a href="7226129829"><code>7226129</code></a> update test workflow to use different artifact names for matrix</li>
<li><a href="ada9446619"><code>ada9446</code></a> update docs and bump <code>@actions/artifact</code></li>
<li><a href="7eafc8b729"><code>7eafc8b</code></a> Merge pull request <a href="https://redirect.github.com/actions/download-artifact/issues/244">#244</a> from actions/robherley/bump-toolkit</li>
<li><a href="3132d12662"><code>3132d12</code></a> consume latest toolkit</li>
<li><a href="5be1d38671"><code>5be1d38</code></a> Merge pull request <a href="https://redirect.github.com/actions/download-artifact/issues/243">#243</a> from actions/robherley/v4-beta-updates</li>
<li><a href="465b526e63"><code>465b526</code></a> consume latest <code>@actions/toolkit</code></li>
<li>Additional commits viewable in <a href="https://github.com/actions/download-artifact/compare/v3...v4">compare view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
</details>
ACKs for top commit:
apoelstra:
ACK 7e2321de75 and will use CI-only one-ack carveout
Tree-SHA512: 8eb6c18ce8c2b99721f134a26e7407c8ebc1d6af4999a013ed395b52795852ddee1c24b1f8aaf60487f13a0c1288d6b496c34d0dbfe20f806fd6fe5643e02250
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.
cb42c74f58 policy: Add section on standard set of derives (Tobin C. Harding)
ec1a5a25c7 CONTRIBUTING: Remove stale links (Tobin C. Harding)
Pull request description:
We can have a standard set of derives to make it easier for new devs to work out what to use and also to help us move towards a uniform set in preparation for crate stabilization.
This is not me imposing my view but rather a place for the discussion to happen, could have been a discussions topic also? Using "open" instead of "draft" to aid visibility. Probably requires more than the usual amount of acks.
ACKs for top commit:
apoelstra:
ACK cb42c74f58
Kixunil:
ACK cb42c74f58
Tree-SHA512: b4c4094ea3652e92a5bea90e16f13971202710166524cc15abef5e8318ba5f59df084f5246331870fc641456b49d4e35d266c937375bdc5035f03699a7d4c1b9
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
4354f37f51 Use NetworkKind in bip32 module (Tobin C. Harding)
35bbfcded7 Use NetworkKind in PrivateKey (Tobin C. Harding)
d22f3828f6 Use NetworkKind in address module (Tobin C. Harding)
6d5ef23e61 Add NetworkKind (Tobin C. Harding)
Pull request description:
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).
Use it in `Address`, `PrivateKey`, and `bip32`.
ACKs for top commit:
Kixunil:
ACK 4354f37f51
apoelstra:
ACK 4354f37f51
Tree-SHA512: d015283b55b2dc68c0a5cc5f17332fce648d3e4fb4e63e0b61943b7edfe4112e8ace760cb743cd3fd5ad0c34636e51a6a12382b34b6e62dfd7dab894d2ceefc0
Andrew Poelstra
dependabot[bot]
Tobin C. Harding
Fmt Bot
GoodDaisy
shuoer86
conduition
yancy
harshit933
Martin Habovstiak