Sync rebase #25
Closed
Sync rebase #25
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The daily fuzzing job is borked error: package `syn v2.0.108` cannot be built because it requires rustc 1.68 or newer, while the currently active rustc version is 1.65.0 Either upgrade to rustc 1.68 or newer, or use cargo update -p [email protected] --precise ver where `ver` is the latest version of `syn` supporting rustc 1.65.0 Use the latest MSRV when fuzzing.
The "lint" and "sane" recipes are kept quick and queit for local dev cycles. The "ci" recipe can be used to match a more extensive CI task locally on failures.
The lint in CI will now fail on duplicate deps. Moved the whitelist to the correct spot so that the current in-progress crates don't fail the check.
This converts the new witness decoder to use the optimized single allocation internal memory structure which was shown back in commit 2fd0125 with the old decoder to be a lot more performant, justifying this increased complexity.
The release script is supposed to do checks if a PR looks like a release tracking PR but it has bugs: - The grep statement is too loose, it catches changes to `rust-version` as well as git context (not just changes). - The grep statement causes the script to exit if it fails. - The crate list is stale. Fix all this so the script does what its supposed to.
601194b Add consensus_encoding to mutation testing (Jamil Lambert, PhD) f7830cf Add mutant exclusions for consensus_encoding (Jamil Lambert, PhD) a8a008f Make mutant exclusions crate specific (Jamil Lambert, PhD) 3af0bb7 Move mutant exclude to correct crate section (Jamil Lambert, PhD) d391298 Add test to kill mutant in decode_from_read (Jamil Lambert, PhD) 20ebf27 Add test to kill mutant in push_bytes (Jamil Lambert, PhD) 671c301 Add test to kill mutant in current_chunk (Jamil Lambert, PhD) Pull request description: `consensus_encoding` is moving towards 1.0 and should be included in the mutation testing with `units` and `primitives`. - First 3 patches add tests to kill existing mutants. - 4th patch moves a mutation exclusion to the correct section in preparation for next patch. - 5th patch prefixes all of the crate specific exclusions with the crate directory so that they are only applied to the intended crate. Also changed `Time` to `LockTime` to be more specific. This reduces the chances of the exclusions hiding other mutants that should be tested. - 6th patch adds a set of exclusions for `consensus_encoding`. The mutation testing mutates operators that are used to check if the decoding or encoding has finished or should continue. These then create a condition where the decoding/encoding never reaches the end. - 7th patch adds `consensus_encoding` to the list of crates to be mutation tested. Closes rust-bitcoin#5205 ACKs for top commit: nyonson: ACK 601194b tcharding: ACK 601194b apoelstra: ACK 601194b; successfully ran local tests; nice!! Tree-SHA512: 740c4c1d700479881b42ca41487608bae31f5fece3ff80803d12dda7e0e1b32f565656f67e8204673e0b931df1bac447eb1149fa3e405eec6fcc2bcf1a3f6a66
e520052 Add as_str method to Opcode (Mitchell Bagot) Pull request description: Currently, serde's serialize_str uses to_string, which requires an expensive allocation on each call. as_str, by definition, produces the same result, but does not require an allocation. Introduce the as_str method in the Opcode implementation and change the serialize_str call to use as_str in place of to_string. Original work by rockcoolsaint in PR rust-bitcoin#4147 This closes rust-bitcoin#4145 ACKs for top commit: apoelstra: ACK e520052; successfully ran local tests tcharding: ACK e520052 Tree-SHA512: f5d8c4ac18c61e67b6b5ff2c6b03ae08e49a2617f1b427f6d0e942726db44358d7ea186d9246b56279642ebe708803ef81dcd6ec7926c2a8ae2d682dae43c371
275c262 ci: add zizmor checks on PRs (Jose Storopoli) 9468b45 ci: add cron-zizmor job (Jose Storopoli) fa3b4f6 ci: ignore zizmor dangerous triggers for PR writers (Jose Storopoli) 594327b ci: fix default permissions to none (Jose Storopoli) bbe1b79 ci: actions/checkout with persist-credentials false (Jose Storopoli) d0508a1 ci: add cooldown to dependabot (Jose Storopoli) 106a8a9 ci: pin actions to tagged hashes (Jose Storopoli) Pull request description: This PR integrates [`zizmor`](https://github.com/zizmorcore/zizmor) for automated GitHub Actions security analysis and implements several workflow security improvements. ## `zizmor` Integration **New workflows** that runs `zizmor`: - Added `.github/workflows/cron-zizmor.yml`: - Currently configured to run daily at midnight UTC - Results are uploaded to the repository's Security tab in SARIF format - Added `.github/workflows/zizmor.yml` that runs `zizmor` on every PR that fails CI if `zizmor` fails. These are the suggested ways to integrate `zizmor` to a repository using GitHub Actions in the documentation. ## CI Security Hardening All workflows were updated with `zizmor`'s security best practices: - **Action pinning**: All GitHub Actions are now pinned to specific commit hashes instead of tags that provide mmutability and protection against supply chain attacks - Example: `actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8` instead of `@v5` - **Permissions**: Added explicit `permissions` blocks to all workflows - Implements least-privilege principle - Each job only requests the minimum permissions needed - **Credential persistence**: Set `persist-credentials: false` on all `actions/checkout` steps - Prevents accidental credential leaks - Reduces attack surface ## Dangerous Triggers Whitelist `zizmor` flagged the use of `pull_request_target` and `workflow_run` in as dangerous trigger patterns. Hence, I've added `# zizmor: ignore[dangerous-triggers]` annotations to whitelist these patterns. ## Dependabot Enhancements - **Cooldown feature**: Added `cooldown.default-days: 5` to `.github/dependabot.yml`. - Enables a "cooldown" period for automated dependabot updates, which restricts dependencies to be updated only if they are more than 5 days old. - Good for supply chain attacks/vulnerabilities. - **Tagged hash support**: Dependabot can work correctly with pinned action versions (commit hashes) and will update them accordingly by pinning a new hash (and changing the commented version accordingly as well). Closes rust-bitcoin#5209. ACKs for top commit: tcharding: ACK 275c262 apoelstra: ACK 275c262; successfully ran local tests Tree-SHA512: abd9279bbec453da6db5fc492e65c64332f0964e2416a34169943745f61c5d7a0a53a81bde0cb7bebe5ac11aca5435c3914e965f7aaa08612f0c62eec1a4c1a6
54ce29f Fuzz with latest MSRV (Tobin C. Harding) Pull request description: The daily fuzzing job is borked error: package `syn v2.0.108` cannot be built because it requires rustc 1.68 or newer, while the currently active rustc version is 1.65.0 Either upgrade to rustc 1.68 or newer, or use cargo update -p [email protected] --precise ver where `ver` is the latest version of `syn` supporting rustc 1.65.0 Use the latest MSRV when fuzzing. ACKs for top commit: apoelstra: ACK 54ce29f; successfully ran local tests; sure Tree-SHA512: b348e7161679ab42c430d96afcd8f4320d42ad32de6060c4c62b33b52ce6ac9af0668bf0a09655d1362df5518d2c8eb75d82c191271325f62354da27f28a6b7f
Bots piss me off. Add a section to the `CONTRIBUTING.md` file with the hope that LLMs will parse it and comply - I have no idea if this is true or not but it is a no-cost attempt at reducing the bot spam we are experiencing.
…ipes f2fcb85 Upgrade maintainer tools and add quiet mode (Nick Johnson) 5abb369 Add back quick and dirty just recipes (Nick Johnson) Pull request description: The "lint" and "sane" recipes are kept quick and quiet for local dev cycles. The "ci" recipe family can be used to match a more extensive CI task locally on failures, but have been namespace'd under `ci-*`. Upgrading maintainer-tools to introduce "quiet mode" as well as fail linting if there are duplicate dependencies. Moved the whitelist to the correct location so the build will pass today with the work in-progress crates. Closes rust-bitcoin#5227 ACKs for top commit: tcharding: tACK f2fcb85 storopoli: tACK f2fcb85 apoelstra: ACK f2fcb85; successfully ran local tests Tree-SHA512: c7237ed89c13fb4140b951b0966dbcd8a11c6b1bf159aa8be725b16367c96c029bbba74511e350fedc59f57410f49db8301dba964d09c32e2097f4d8c1b2c5c7
… file ed8414c Add a section about LLMs to the contributing file (Tobin C. Harding) Pull request description: Bots piss me off. Add a section to the `CONTRIBUTING.md` file with the hope that LLMs will parse it and comply - I have no idea if this is true or not but it is a no-cost attempt at reducing the bot spam we are experiencing. ACKs for top commit: apoelstra: ACK ed8414c; successfully ran local tests jamillambert: ACK ed8414c storopoli: ACK ed8414c jrakibi: ACK ed8414c Tree-SHA512: 84a2d281e613fe87b0ceaae3336899ebfaec27bca240b5803c88ac0b7295b0ca9aed57f24ac24905377b4e0b6a32259dbc19b04a006b8e2f6a1531a669b289da
a7379e3 consensus_encoding: Introduce API text files (Jamil Lambert, PhD) 7b1c401 Bump cargo-public-api to 0.50.1 in CI (Jamil Lambert, PhD) Pull request description: In preparation for 1.0-ing `consensus_encoding` update the API checker script to include `consensus_encoding` and introduce the API text files. The current CI workflow `cargo-public-api` is pinned to version 0.49.0. This results in an ```unknown variant `macro_export` ``` error when checking the API files of `consensus_encoding`. Bump the `cargo-public-api` version in CI to 0.50.1 ACKs for top commit: nyonson: ACK a7379e3 tcharding: ACK a7379e3 apoelstra: ACK a7379e3; successfully ran local tests Tree-SHA512: a20edac67ad21186f330ddfbdbf04570dd188ba9a9a0aa823e8e259641be095c154e40a3070a73a97dc3c7facaece94c7172207be43e03f3ea3dc36e1b6ec337
…1.91.0) 0001850 Automated update to Github CI to rustc stable-1.91.0 (Update Stable Rustc Bot) Pull request description: Automated update to Github CI workflow `semver-checks.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action ACKs for top commit: tcharding: ACK 0001850 Tree-SHA512: 4998df11c0afc77fca60fa94d555666f91985f09645d12cfd6f16ba77e7e75e9bff3f4ae72eb10ad30d47a3db7bb2aa427a44158dee91ebd729aa13769f8df81
df64705 primitives: single allocation witness decoder (Nick Johnson) Pull request description: When attempting to simplify the `WitnessDecoder` in rust-bitcoin#5224, we noticed that we were double allocating which erased the performance gains of the old decoder from rust-bitcoin#672. This updates `WitnessDecoder` to use a single allocation like the old decoder. While the intention of this patch was purely for performance, three of the new tests fail against the current version of the `WitnessDecoder`: `decode_empty_element`, `decode_multiple_empty_elements`, and `decode_incomplete_witness_count`. For the first two tests, there is a functional change in how empty witness elements are handled. This patch introduces a short-circuit where the old version might get stuck in a loop forever requesting zero bytes. The third test, `decode_incomplete_witness_count`, shows that the impl has a more explicit failure now if `end` is called before the witness element count is complete, instead of just returning and empty witness. ACKs for top commit: tcharding: ACK df64705 jrakibi: ACK df64705 apoelstra: ACK df64705; successfully ran local tests Tree-SHA512: 5177410da2d3ceb65a7bfc3e8444a913d23cad1c8e83f045501882b45bac2a90f403d0f7e72fe6b88c25774bbf636e007726d0dc93aeb77bed4ec94d67efa2bb
…x warnings 5ecef4b refactor: enable redundant_clone lint and fix warnings (Klaklax) Pull request description: Enabled redundant_clone in all crates and fixed the leftover clones ACKs for top commit: tcharding: ACK 5ecef4b storopoli: ACK 5ecef4b apoelstra: ACK 5ecef4b; successfully ran local tests Tree-SHA512: e0d89d8002f0fe3d7a8c2a49dc1e08fe19f6bd3780a18cee9039610fdf95e8f417af3dc150ede8c616767f7b1e76d72c84e01076bd90074bf5d1599b76a771fb
While adding API text files for `consensus_encoding` we also had a PR in flight that change the API (making a bunch of functions const). Update the API text files, no manual changes.
a3b9108 Run the API checker script (Tobin C. Harding) Pull request description: While adding API text files for `consensus_encoding` we also had a PR in flight that change the API (making a bunch of functions const). Update the API text files, no manual changes. ACKs for top commit: apoelstra: ACK a3b9108; successfully ran local tests Tree-SHA512: 1227d8335a33b1276a395c90029dfb8b2e096f1651efbd6c6649b29e01e6fec3c17033ccd71856b78774fb1b6896989a0e00b4be2aef66bf58f649e5a684de76
5a6d089 CI: Fix release script (Tobin C. Harding) Pull request description: The release script is supposed to do checks if a PR looks like a release tracking PR but it has bugs: - The grep statement is too loose, it catches changes to `rust-version` as well as git context (not just changes). - The grep statement causes the script to exit if it fails. - The crate list is stale. Fix all this so the script does what its supposed to. ACKs for top commit: storopoli: tACK 5a6d089 apoelstra: ACK 5a6d089; successfully ran local tests Tree-SHA512: bd43769dc1492f31fef067f1353c040fb212b8fc93ffa8ac863a87489010dca9990b74df4f5b15135c3ee60e874d238de9f6535445edbb3405f8bf2064a51f33
Mutation testing changes a *= to += in the function that increases the buffer size when needed. The difference is purely performant and untestable. Exclude the mutant.
Upgrade to the latest released version of `rust-secp256k1`. Do just the build errors. Note one deprecated call to `thread_rng()` is added, I'm not sure why this line was building before but anyways the `rand` stuff will be done next.
For an attempt to catch bots/low-quality contributions we add AGENTS.md that tells LLMs to identify themselves
The docs on internals::impl_to_hex_from_lower_hex macro state that the calling crate must have an "alloc" feature. If this feature is not enabled, the macro is a no-op. In practice, the macro has no such feature gate, as it is primarily used in the bitcoin crate, which has no "alloc" feature. Adjust the wording of the internals::impl_to_hex_from_lower_hex macro to clarify the true behaviour.
81a91f4 doc: Add agents.md (jrakibi) Pull request description: As an attempt to catch bots/low-quality contributions we add AGENTS.md that tells LLMs to identify themselves. disclosed via commit message: `> assisted-by-llm ` ACKs for top commit: apoelstra: ACK 81a91f4; successfully ran local tests Tree-SHA512: a818c00eea22d980b6d018f42ba2c16da56e52239e596c237319a2802f140892bd842da9ba3f977d219de1942fc96ebdae438237a4d4860f07851a1a94fe5d0d
…ection on README 7061fdb chore(docs): remove `act` from CI Pipeline section on README (Luis Schwab) Pull request description: Remove act from README since it was removed in rust-bitcoin#4533. ACKs for top commit: apoelstra: ACK 7061fdb; successfully ran local tests Tree-SHA512: 2459031448718793b1fb69fe0676336d501433240cb3f46263884d73dfc8e10f76e69e1cdf9d1fcde2e58b435bd71d0fb5d9defd507ded0fa780dd95b2eedd2f
- Remove rand deprecated stuff - Use new Keypair API - Remove unnecessary import
Cleans up the warning
…_with_limit 4f80f6d Use usize for CompactSizeDecoder and add new_with_limit (Mitchell Bagot) Pull request description: Following the change to CompactSizeEncoder::new to take a usize, the return types and ranges of the CompactSizeEncoder and CompactSizeDecoder now differ. Since a device's memory addressing is limited to usize, returning values beyond that size is largely useless. Converting the CompactSizeDecoder to return a usize also allows the cast_to_usize_if_valid function to be merged into the decoder itself. Convert the CompactSizeDecoder to return a usize. Remove cast_to_usize_if_valid and introduce its functionality into CompactSizeDecoder. Follow up to rust-bitcoin#5344 ACKs for top commit: tcharding: ACK 4f80f6d apoelstra: ACK 4f80f6d; successfully ran local tests Tree-SHA512: 970eface53938c939b3bdbb7a9d89df43bce8acfdd69b6eaba2e9be4ec65eb03179bfe2daa7742176bdb285b236fdbb743ef2d227168ac66d73a33e126ffd0d8
9a11819 primitives: Remove arrayvec dependency (Mitchell Bagot) Pull request description: The primitives crate only uses the arrayvec crate to provide Display trait for Header. In order to minimise the use of external dependencies, it should be removed. Convert Header Display impl to use fmt_hex_exact and remove the arrayvec dependency from primitives. ACKs for top commit: apoelstra: ACK 9a11819; successfully ran local tests tcharding: ACK 9a11819 Tree-SHA512: e7305af3c51885b2fc07fcb884acb5b54403a8ab391e2d7b47fc8fc05e0ccd9c12622b7a0e8c0676ed9ee424f49561d17889198fcb39a300e631c19af51b5b24
d893038 p2p: upgrade to workspace lint rules (Nick Johnson) d8de11d internals: upgrade to workspace lint rules (Nick Johnson) Pull request description: These include all the clippy recommendations for the new rules. I had to ask the LLM the "but why" for a handful of these, but I think they all check out. ACKs for top commit: apoelstra: ACK d893038; successfully ran local tests Tree-SHA512: 4749f19e32beaf4c7901f7d035f41d7317b76aaa3b4a20a9d6f09332e280081a69063f1891bb7698f8952e0a2b9680722da3ec29ec0dc1c1bd466514c365541a
…e type definitions overlap c97d458 Add re-export for primitives to Bitcoin crate (yancy) f81e8d2 Depend on primitives instead of bitcoin where type definitions overlap (yancy) Pull request description: Consolidate dependency tree for downstream crates that depend on `bitcoin-p2p-messages` by reducing overlapping dependencies. For example, consider crate A which depends on `bitcoin-primitives` and `bitcoin-p2p-messages`. For types defined by both `bitcoin-primitives` and `bitcoin`, Crate A may still need to depend on `bitcoin` if a type of `bitcoin-p2p-messages` depends on `bitcoin` instead of `bitcoin-primitives`. Also, see: rust-bitcoin#5331 _After_ this change, the remaining bitcoin dependencies are: ``` examples/handshake.rs:6:use bitcoin::consensus::{encode, Decodable}; src/address.rs:15:use bitcoin::consensus::encode::{self, Decodable, Encodable, ReadExt, WriteExt}; src/address.rs:532: use bitcoin::consensus::encode::{deserialize, serialize}; src/consensus.rs:2:use bitcoin::consensus::encode::WriteExt; src/lib.rs:43:use bitcoin::consensus::encode::{self, Decodable, Encodable}; src/lib.rs:44:use bitcoin::network::{Network, Params, TestnetVersion}; src/lib.rs:489: use bitcoin::consensus::encode::{deserialize, serialize}; src/message.rs:17:use bitcoin::block::HeaderExt; src/message.rs:18:use bitcoin::consensus::encode::{self, Decodable, Encodable, ReadExt, WriteExt}; src/message.rs:19:use bitcoin::merkle_tree::MerkleBlock; src/message.rs:959: use bitcoin::bip152::BlockTransactionsRequest; src/message.rs:960: use bitcoin::bip158::{FilterHash, FilterHeader}; src/message.rs:962: use bitcoin::consensus::encode::{deserialize, deserialize_partial, serialize}; src/message_blockdata.rs:13:use bitcoin::consensus::encode::{self, Decodable, Encodable}; src/message_blockdata.rs:175: use bitcoin::consensus::encode::{deserialize, serialize}; src/message_bloom.rs:11:use bitcoin::consensus::{encode, Decodable, Encodable, ReadExt}; src/message_compact_blocks.rs:8:use bitcoin::bip152; src/message_filter.rs:11:use bitcoin::bip158::{FilterHash, FilterHeader}; src/message_network.rs:15:use bitcoin::consensus::{encode, Decodable, Encodable, ReadExt, WriteExt}; src/message_network.rs:420: use bitcoin::consensus::encode::{deserialize, serialize}; src/network_ext.rs:7:use bitcoin::{Network, TestnetVersion}; ``` I've left bip152 and bip158 for future PRs (rust-bitcoin#5333). I don't think any of the remaining dependencies can be readily moved. ACKs for top commit: apoelstra: ACK c97d458; successfully ran local tests tcharding: ACK c97d458 Tree-SHA512: 389eb09d8b3a75de0cb30baa3722c9e7b03273dfb8547211246349e97ab205c66354aaed2a49415faf6e01fef41a496d114fb68869355f95778f69e2346bb8c6
f8d623e Add missing serde tests for units (Jakub Gladysz) b7bbee8 Rename serde tests in units (Jakub Gladysz) 8912345 Rename struct fields in units serde test (Jakub Gladysz) 197968b Remove redundant import from sequence.rs (Jakub Gladysz) Pull request description: related to [this discussion](rust-bitcoin#4921 (comment)) Sorry it took so long but after this discussion I immediately got flooded with some other projects, but I never aimed to give up on this. So as instructed, starting with `units` first I wanted to check if this is what is expected (hence draft). If true, then I will add the last missing one in for `locktime`. From added tests, all besides `FeeRate` might be seen as redundant, because `serde_regression` is covering them, but this is how I understood 'human-readable' ones you mentioned as opposed to reading from the file. Then I will have a question about "fuzz test serde logic for types in units" - do you have any example test from other modules that I can get inspired from? ACKs for top commit: apoelstra: ACK f8d623e; successfully ran local tests; thanksgit show pr/5409/head~0 Tree-SHA512: 9d5293aa2fbac8c787fe5da0a2ea9f291586c7f1e17c1eaef58734b8629dd54f12f5039166ac73d06f17d419b350f8592d5a9bfe7fe10aa1673f06d150ec1225
…imitives crate f44e8b8 docs: add missing errors documentation to primitives crate (Alkamal01) Pull request description: This PR addresses the `TODO` in [primitives/src/lib.rs](cci:7://file:///home/kaftandev/cont/primitives/src/lib.rs:0:0-0:0) by enabling the `clippy::missing_errors_doc` lint for the `bitcoin-primitives` crate. It adds the missing `# Errors` section to the documentation of: - `ScriptHash::from_script` - `WScriptHash::from_script` - `Block::validate` These changes ensure better documentation coverage and help consumers understand potential error conditions. ## Checklist - [x] All new code is covered by unit tests (docs only changes) - [x] All changes compile and pass tests locally - [x] Documentation updated ACKs for top commit: apoelstra: ACK f44e8b8; successfully ran local tests Tree-SHA512: 121cddb7723fcdef23f0974d3b92db4c06c5aef2cb81046d2a40838ce61af63de41c2a18208dc65f9a0008bdabf4f19503108f71b0ce2a040f4a740aec80baca
1da9343 2025-12-21 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: apoelstra: ACK 1da9343; successfully ran local tests; this is more like it Tree-SHA512: b10e4dea329e564bc3f5f73351b4245dfe8d349ee2cdd9181dbe4e40f530dd5df6c3d8cfa79af24dca1adf5f89864977c90e0d94138df7a0dac32536a5dcffb4
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
19 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.