Skip to content

ci: make the new static-scan jobs (rustfmt/audit/deny) pass - #442

Open
chee-chyuan wants to merge 4 commits into
feat/static-code-scanfrom
fix/static-scan-ci-green
Open

chee-chyuan wants to merge 4 commits into
feat/static-code-scanfrom
fix/static-scan-ci-green

Conversation

@chee-chyuan

Copy link
Copy Markdown
Contributor

Summary

#439 introduces three informational CI jobs that all start red because they surface pre-existing repo debt. This PR (targeting the #439 branch) clears all three so the gates start green and any new red actually means something.

  • Rustfmtcargo +nightly-2026-05-11 fmt --all across the workspace (same pinned toolchain as CI; 104 files, formatting only). Also strips pre-existing trailing whitespace in provider.rs/main.rs that made rustfmt abort with internal errors.
  • cargo-audit — bumps the only two advisories fixable within semver (crossbeam-epoch 0.9.20, quinn-proto 0.11.16). The remaining 9 advisories are pinned by git deps (tendermint-rs-parlia, greenfield-cometbft-rs, the reth rev) and need semver-major upstream bumps; they are acknowledged in a new .cargo/audit.toml with per-entry justifications.
  • cargo-deny — bumps lru 0.12 → 0.16.3 (real fix for RUSTSEC-2026-0002, direct dep, zero code changes); mirrors the pinned-vulnerability ignores into deny.toml plus 7 unmaintained transitive crates with their pinning chains documented; allowlists MPL-2.0 for bitmaps/contracts/imbl/imbl-sized-chunks.

⚠️ Reviewer decision needed: the MPL-2.0 allowlisting reverses the deliberate "flag for manual review" placeholder in deny.toml. MPL-2.0 is weak file-level copyleft — linking unmodified crates into the binary imposes nothing — but it should get an explicit sign-off here.

The ignore lists are per-advisory, not per-crate: new advisories against the same crates still fail CI. Both files say to re-check the lists whenever one of the pinned git deps is bumped.

Test plan

  • cargo +nightly-2026-05-11 fmt --all -- --check → clean (also re-verified after a build, so the generated embedded_contracts.rs doesn't reintroduce drift)
  • cargo audit → exit 0 (16 pre-existing unmaintained/yanked warnings remain informational)
  • cargo deny --all-features check → advisories ok, bans ok, licenses ok, sources ok
  • cargo check → clean
  • cargo test --all -- --test-threads=1 → 304 passed, 0 failed

🤖 Generated with Claude Code

chee-chyuan and others added 3 commits July 21, 2026 15:42
Makes the new (non-blocking) Rustfmt CI job pass. Formatting-only: applied
cargo +nightly-2026-05-11 fmt --all (same pinned toolchain as CI), plus
removed pre-existing trailing whitespace in provider.rs/main.rs that made
rustfmt bail with internal errors. No functional changes; cargo check passes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…isories

cargo update -p crossbeam-epoch (0.9.18 -> 0.9.20, RUSTSEC-2026-0204) and
-p quinn-proto (0.11.14 -> 0.11.16, RUSTSEC-2026-0185), the only two
advisories fixable within semver.

The remaining 9 advisories are all pinned by git deps (tendermint-rs-parlia,
greenfield-cometbft-rs, the reth rev) and need semver-major upstream bumps;
list them in .cargo/audit.toml ignore with per-entry justifications so the
job goes green and any NEW advisory fails it again.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…llow MPL-2.0

- lru 0.12 -> 0.16.3: real fix for RUSTSEC-2026-0002 (IterMut unsoundness);
  direct dep, compiles with no code changes, full test suite passes.
- deny.toml advisories: ignore the 8 vulnerabilities pinned by git deps
  (tendermint-rs-parlia, greenfield-cometbft-rs, the reth rev) — mirrors
  .cargo/audit.toml — plus 7 unmaintained transitive crates, each with the
  pinning chain documented.
- deny.toml licenses: allow MPL-2.0 (weak file-level copyleft, fine for
  linking unmodified crates: bitmaps, contracts, imbl, imbl-sized-chunks).

cargo deny --all-features check: advisories ok, bans ok, licenses ok, sources ok.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@chee-chyuan
chee-chyuan requested a review from joey0612 as a code owner July 21, 2026 08:12
@hashdit-bot

hashdit-bot Bot commented Jul 21, 2026

Copy link
Copy Markdown

Pull Request Review

This Rust-based BSC node PR applies workspace-wide rustfmt cleanup and updates lru, crossbeam-epoch, and quinn-proto to address fixable advisories. It also adds cargo-audit/cargo-deny exceptions for vulnerabilities pinned by upstream Git dependencies and permits MPL-2.0 dependencies.

Sensitive Content

No sensitive content detected.

Security Issues

No serious security issues detected.


Generated by Hashdit Bot. This tool can absolutely NOT replace manual audits.

let vote_buffer = self
.cur_votes
.entry(vote_address)
.or_insert_with(|| LruCache::new(NonZero::new(MAX_SIZE_OF_RECENT_ENTRY).unwrap()));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ [clippy] reported by reviewdog 🐶

warning: used `unwrap()` on an `Option` value
  --> src/consensus/parlia/malicious_vote_monitor.rs:52:46
   |
52 |             .or_insert_with(|| LruCache::new(NonZero::new(MAX_SIZE_OF_RECENT_ENTRY).unwrap()));
   |                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: if this value is `None`, it will panic
   = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.97.0/index.html#unwrap_used

Comment thread src/consensus/parlia/provider.rs Outdated
tracing::trace!("Succeed to walk to parent block, parent_block_number: {}", parent_header.clone().unwrap().number);
tracing::trace!(
"Succeed to walk to parent block, parent_block_number: {}",
parent_header.clone().unwrap().number

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ [clippy] reported by reviewdog 🐶

warning: used `unwrap()` on an `Option` value
   --> src/consensus/parlia/provider.rs:280:21
    |
280 |                     parent_header.clone().unwrap().number
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: if this value is `None`, it will panic
    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.97.0/index.html#unwrap_used

Comment thread src/consensus/parlia/provider.rs Outdated
base_snapshot.clone().unwrap().block_number, target_header.number, rebuild_block_hashes.len());
tracing::debug!(
"try rebuild snapshot, from_block: {}, to_block: {}, rebuild_block_len: {:?}",
base_snapshot.clone().unwrap().block_number,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ [clippy] reported by reviewdog 🐶

warning: used `unwrap()` on an `Option` value
   --> src/consensus/parlia/provider.rs:291:13
    |
291 |             base_snapshot.clone().unwrap().block_number,
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: if this value is `None`, it will panic
    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.97.0/index.html#unwrap_used

Comment thread src/consensus/parlia/validation.rs Outdated
if self.spec.is_bohr_active_at_timestamp(header.number, header.timestamp) {
if header.parent_beacon_block_root.is_none() ||
header.parent_beacon_block_root.unwrap() != B256::default()
header.parent_beacon_block_root.unwrap() != B256::default()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ [clippy] reported by reviewdog 🐶

warning: used `unwrap()` on an `Option` value
   --> src/consensus/parlia/validation.rs:216:17
    |
216 |                 header.parent_beacon_block_root.unwrap() != B256::default()
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: if this value is `None`, it will panic
    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.97.0/index.html#unwrap_used

Comment thread src/node/vote_journal.rs
/// Get a guard to the global vote journal.
pub fn global() -> std::sync::MutexGuard<'static, VoteJournal> { GLOBAL_JOURNAL.lock().expect("vote journal poisoned") }
pub fn global() -> std::sync::MutexGuard<'static, VoteJournal> {
GLOBAL_JOURNAL.lock().expect("vote journal poisoned")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ [clippy] reported by reviewdog 🐶

warning: used `expect()` on a `Result` value
   --> src/node/vote_journal.rs:189:5
    |
189 |     GLOBAL_JOURNAL.lock().expect("vote journal poisoned")
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: if this value is an `Err`, it will panic
    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.97.0/index.html#expect_used
note: the lint level is defined here
   --> src/node/vote_journal.rs:1:30
    |
  1 | #![warn(clippy::unwrap_used, clippy::expect_used)]
    |                              ^^^^^^^^^^^^^^^^^^^

…gate

The rustfmt commit reformatted lines inside modules where #439 adds
#![warn(clippy::unwrap_used, clippy::expect_used)], so filter_mode=added
reported 11 pre-existing sites. Production sites get real fixes:

- provider.rs try_rebuild: let-else bindings replace is_none/unwrap and
  drop four redundant clones per loop iteration
- validation.rs: parent_beacon_block_root check collapsed to a direct
  Option comparison (same truth table, no unwrap)
- malicious_vote_monitor.rs: MAX_SIZE_OF_RECENT_ENTRY becomes a
  compile-time-checked NonZero<usize> const
- vote_journal.rs global(): expect kept deliberately (poisoned journal ->
  double-signing risk), justified with #[allow] per the gate's policy

Test modules get a blanket #![allow] — panicking is the assertion
mechanism in tests.

cargo fmt --check clean; reth_bsc lib tests 304 passed / 0 failed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hashdit-bot

hashdit-bot Bot commented Jul 21, 2026

Copy link
Copy Markdown

Pull Request Review

This Rust-based BSC blockchain node PR updates lru, crossbeam-epoch, and quinn-proto, adds cargo-audit and cargo-deny exceptions for pinned advisories, and allowlists MPL-2.0 dependencies. It also applies workspace-wide Rust formatting and minor formatting-driven refactors without intended behavioral changes.

Sensitive Content

No sensitive content detected.

Security Issues

No serious security issues detected.


Generated by Hashdit Bot. This tool can absolutely NOT replace manual audits.

Comment thread src/evm/api/mod.rs
let mismatched_result = mismatched
.transact_one(tx)
.expect("execution should not error");
let mismatched_result = mismatched.transact_one(tx).expect("execution should not error");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ [Semgrep OSS] <semgrep.high-risk-unwrap> reported by reviewdog 🐶
A new unwrap() or expect() call was added in a high-risk module. Please reply directly to this comment explaining "why unwrap cannot panic here"; once the code owner confirms, the conversation can be resolved


let resolved =
super::resolve_fcu_head_header(&provider, side_hash, None, None).unwrap();
let resolved = super::resolve_fcu_head_header(&provider, side_hash, None, None).unwrap();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ [Semgrep OSS] <semgrep.high-risk-unwrap> reported by reviewdog 🐶
A new unwrap() or expect() call was added in a high-risk module. Please reply directly to this comment explaining "why unwrap cannot panic here"; once the code owner confirms, the conversation can be resolved

Comment thread src/node/vote_journal.rs
// A poisoned lock means a writer panicked mid-update; continuing with possibly
// inconsistent vote records risks double-signing, so crashing is the safe choice.
#[allow(clippy::expect_used)]
GLOBAL_JOURNAL.lock().expect("vote journal poisoned")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ [Semgrep OSS] <semgrep.high-risk-unwrap> reported by reviewdog 🐶
A new unwrap() or expect() call was added in a high-risk module. Please reply directly to this comment explaining "why unwrap cannot panic here"; once the code owner confirms, the conversation can be resolved

Comment thread src/node/vote_journal.rs
.unwrap()
.as_nanos();
let ts =
std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ [Semgrep OSS] <semgrep.high-risk-unwrap> reported by reviewdog 🐶
A new unwrap() or expect() call was added in a high-risk module. Please reply directly to this comment explaining "why unwrap cannot panic here"; once the code owner confirms, the conversation can be resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant