Skip to content

fix: replace H160::from_slice().unwrap() with proper error handling#666

Open
eval-exec wants to merge 3 commits into
nervosnetwork:developfrom
eval-exec:fix/h160-from-slice-unwrap
Open

fix: replace H160::from_slice().unwrap() with proper error handling#666
eval-exec wants to merge 3 commits into
nervosnetwork:developfrom
eval-exec:fix/h160-from-slice-unwrap

Conversation

@eval-exec

Copy link
Copy Markdown
Collaborator

Summary

Replace all 32 instances of H160::from_slice(...).unwrap() across the codebase with proper error handling.

Problem

H160::from_slice() returns Result<H160, String>, but the codebase had 32 sites where .unwrap() was called on this result. These would panic/crash the CLI if:

  • An RPC node returned an address with a malformed payload
  • User input contained an address with args shorter than 20 bytes
  • Malformed data was encountered during transaction building

This was the highest-frequency panic pattern identified in the codebase audit.

Changes

Category Approach
Functions returning Result .map_err(
Closures inside .map() on Result Convert to .and_then()
Closures inside .map() on Option .and_then() + .ok()
format!() calls Extract H160 to variable first
Guarded sites in signer.rs .expect() with clear message
[0..20] slicing Add bounds check via .get(0..20)

Files Changed

  • src/subcommands/account.rs — 1 fix
  • src/subcommands/dao/command.rs — 1 fix
  • src/subcommands/deploy/mod.rs — 2 fixes
  • src/subcommands/sudt.rs — 8 fixes
  • src/subcommands/tx.rs — 3 fixes
  • src/subcommands/util.rs — 4 fixes
  • src/subcommands/wallet.rs — 5 fixes
  • src/utils/signer.rs — 5 fixes
  • src/utils/tx_helper.rs — 4 fixes

Verification

  • cargo check — passes
  • cargo nextest run — all 7 tests pass
  • grep -rn 'H160::from_slice.*.unwrap()' src/ — zero results

Replace all 32 instances of H160::from_slice(...).unwrap() across the
codebase with proper error handling:

- For functions returning Result: use .map_err(|e| format!(...))?
- For closures inside .map() on Result: convert to .and_then()
- For closures inside .map() on Option: convert to .and_then().ok()
- For format!() calls: extract H160 to a variable first
- For guarded/safe sites in signer.rs: use .expect() with clear message
- For [0..20] slicing: add bounds check via .get(0..20) before H160

Fixes the highest-frequency panic pattern in the codebase (~30 sites
that could crash from malformed address data from RPC/user input).
@eval-exec eval-exec force-pushed the fix/h160-from-slice-unwrap branch from 59f2145 to b3f0930 Compare June 18, 2026 03:16
@doitian

doitian commented Jun 18, 2026

Copy link
Copy Markdown
Member

Consider adding a helper function.

Extract the repeated pattern of H160::from_slice(x).map_err(...)
into two helper functions in src/utils/other.rs:

- h160_from_slice(data, context) -> Result<H160, String>
- h160_from_slice_prefix(data, context) -> Result<H160, String>
  (takes first 20 bytes, with bounds check)

Replace all inline H160::from_slice error handling across 8 files
with calls to these helpers. Net reduction of 7 lines while making
error messages consistent and code more readable.
@eval-exec eval-exec requested review from chenyukang and quake June 18, 2026 05:08
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.

3 participants