Skip to content

Add and apply most of rust-bitcoin's lints#140

Open
luisschwab wants to merge 5 commits into
mit-dci:mainfrom
luisschwab:chore/add-rust-bitcoin-lints
Open

Add and apply most of rust-bitcoin's lints#140
luisschwab wants to merge 5 commits into
mit-dci:mainfrom
luisschwab:chore/add-rust-bitcoin-lints

Conversation

@luisschwab

@luisschwab luisschwab commented Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Changelog

- Bump `edition` to 2021 (supported since Rust 1.56.0)
- Pins `cargo-audit` and `zizmor` versions on `Cargo.toml`, such that local and CI use the same versions
- Implement `new_hash_map` and `new_hash_set` for the `std` prelude
- Add and apply all of `rust-bitcoin`'s lints, except for documentation lints, left to a follow-up
- Add the `#![no_std]` directive such that `cargo-rbmt` will also test on a no-std target

@luisschwab
chore: bump edition to 2021 and update Cargo.toml
846a6d1 
* Bump the Rust edition to 2021 (released on Rust 1.56.0)
* Add the `tools` field to `[package.metadata.rbmt]` such that
  CI and local `cargo-audit` and Zizmor versions are in sync
* Update the Zizmor CI job to set up rbmt's tools
* Update the justfile and README
@luisschwab
luisschwab force-pushed the chore/add-rust-bitcoin-lints branch from c2e655d to de8c93a Compare July 8, 2026 20:53
@luisschwab

luisschwab commented Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Added the #![no_std] directive so that cargo-rbmt will also test on a no-std target. This way we can drop the test-no-std CI job.

~$ cargo rbmt test

[...]

Tests complete.
Test Summary
Commit: de8c93a0b0bdd4864cb512bd5344a81a925cf2a2
  Package: rustreexo
    Examples           : stump_modify, stump_modify:std, stump_modify:with-serde, stump_modify:with-serde std, proof_update, proof_update:std, proof_update:with-serde, proof_update:with-serde std, custom_hash, custom_hash:std, custom_hash:with-serde, custom_hash:with-serde std
    Individual features: serde, std, with-serde
    Feature subsets    : [serde, std], [serde, with-serde], [std, with-serde]
+   No-std check       : ran

ref: https://git.rust-bitcoin.org/rust-bitcoin/rust-bitcoin-maintainer-tools/src/branch/master/cargo-rbmt#no_std

@luisschwab
luisschwab force-pushed the chore/add-rust-bitcoin-lints branch from 1bdc74d to 243e3cb Compare July 8, 2026 21:34
@luisschwab luisschwab mentioned this pull request Jul 8, 2026
Merged
Davidson-Souza
Davidson-Souza requested changes Jul 16, 2026
View reviewed changes
Comment thread justfile Outdated
cargo rbmt test --toolchain msrv --lockfile minimal

[doc: "Install cargo-rbmt tools"]
tools:

@Davidson-Souza Davidson-Souza Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: install-tools?

@luisschwab luisschwab Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think setup or bootstrap are better names, what do you think?

Comment thread .github/workflows/audit.yml Outdated
Comment thread src/lib.rs
Comment thread src/pollard/mod.rs Outdated
Comment thread src/pollard/mod.rs Outdated
Comment thread src/proof/mod.rs
Comment on lines 736 to 737
cached_del_hashes: Vec<Hash>,
remembers: Vec<u64>,

@Davidson-Souza Davidson-Souza Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

de8c93a

Why not making the others a reference as well?

@luisschwab luisschwab Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just fixed whatever clippy complained about, we can go over all param types on a follow-up.

Comment thread src/proof/mod.rs
util::get_proof_positions(&new_target_pos, num_leaves, util::tree_rows(num_leaves));
needed_proof_positions.sort();
get_proof_positions(&new_target_pos, num_leaves, tree_rows(num_leaves));
needed_proof_positions.sort_unstable();

@Davidson-Souza Davidson-Souza Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

de8c93a
Hmm, I wonder if this could have implications. Is there a way were we could screw up the order?

@luisschwab luisschwab Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine, given need_proof_positions is a stable primitive (Vec<u64>).

error: used `sort` on primitive type `u64`
   --> src/proof/mod.rs:792:9
    |
792 |         needed_proof_positions.sort();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `needed_proof_positions.sort_unstable()`
    |
    = note: an unstable sort typically performs faster without any observable difference for this data type
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
    = note: `-D clippy::stable-sort-primitive` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::stable_sort_primitive)]`

Comment thread src/proof/mod.rs
Comment thread Cargo.toml
trivially_copy_pass_by_ref = "warn"
unchecked_time_subtraction = "warn"
unicode_not_nfc = "warn"
uninlined_format_args = "allow" # This is a subjective style choice.

@Davidson-Souza Davidson-Souza Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually like this one

@luisschwab luisschwab Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which one?

Comment thread Cargo.toml Outdated
@luisschwab
feat!: implement new_hash_map and new_hash_set for the std prelude
a6368d7 
The `default_trait_access` lint complained that we were instantiating
a `HashMap` with `HashMap::with_hasher(Default::default())`.

I've mirrored the helpers on the `std` prelude
such that we may keep the two in sync.
@luisschwab
fix: make cargo-rbmt run #![no_std] tests
b523717 
Import the `std` crate, feature gated on the `std` feature,
and add the `#![no_std]` attribute such that `cargo-rbmt`
also tests the crate in a no-std target (`thumbv7m-none-eabi`).
@luisschwab
chore!: add and apply most of rust-bitcoin's lints
0ecf725 
Add and apply all `rust-bitcoin` lints, except for documentation lints.
@luisschwab
chore(ci): move cargo rbmt tools to vendored action
425181b
@luisschwab
luisschwab force-pushed the chore/add-rust-bitcoin-lints branch from 243e3cb to 425181b Compare July 17, 2026 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Davidson-Souza Davidson-Souza Awaiting requested review from Davidson-Souza

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@luisschwab @Davidson-Souza