diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 4dff25a7..9aa01f3f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -21,9 +21,7 @@ jobs: os: [ ubuntu-latest, macos-15-intel, windows-latest ] steps: - uses: actions/checkout@v3 - - uses: dtolnay/rust-toolchain@stable - with: - toolchain: 1.95.0 + - uses: actions-rust-lang/setup-rust-toolchain@v1 - if: matrix.os == 'windows-latest' name: Windows Dependencies shell: pwsh @@ -42,9 +40,7 @@ jobs: os: [ ubuntu-latest, macos-15-intel ] steps: - uses: actions/checkout@v2 - - uses: dtolnay/rust-toolchain@stable - with: - toolchain: 1.95.0 + - uses: actions-rust-lang/setup-rust-toolchain@v1 - name: Integration_Test run: make integration @@ -55,13 +51,11 @@ jobs: os: [ ubuntu-latest, macos-15-intel ] steps: - uses: actions/checkout@v2 - - uses: dtolnay/rust-toolchain@stable + - uses: actions-rust-lang/setup-rust-toolchain@v1 with: - toolchain: 1.95.0 + components: rustfmt, clippy - name: Linters run: | - cargo fmt --version || rustup component add rustfmt - cargo clippy --version || rustup component add clippy make fmt make clippy git diff --exit-code Cargo.lock @@ -76,9 +70,7 @@ jobs: - --hide-inclusion-graph --show-stats licenses steps: - uses: actions/checkout@v3 - - uses: dtolnay/rust-toolchain@stable - with: - toolchain: 1.95.0 + - uses: actions-rust-lang/setup-rust-toolchain@v1 - name: cargo-deny uses: EmbarkStudios/cargo-deny-action@v2 with: diff --git a/src/subcommands/account.rs b/src/subcommands/account.rs index b26ea69f..bfe51c6f 100644 --- a/src/subcommands/account.rs +++ b/src/subcommands/account.rs @@ -20,7 +20,7 @@ use crate::utils::{ ArgParser, ExtendedPrivkeyPathParser, FilePathParser, FixedHashParser, FromStrParser, HexParser, PrivkeyPathParser, PrivkeyWrapper, }, - other::{address_json, read_password}, + other::{address_json, h160_from_slice, read_password}, }; pub struct AccountSubCommand<'a> { @@ -493,8 +493,9 @@ impl CliSubCommand for AccountSubCommand<'_> { .keystore_handler() .extended_pubkey(lock_arg, &path, password)?; let address_payload = AddressPayload::from_pubkey(&extended_pubkey); + let lock_arg = h160_from_slice(address_payload.args().as_ref(), "address payload")?; let resp = serde_json::json!({ - "lock_arg": format!("{:#x}", H160::from_slice(address_payload.args().as_ref()).unwrap()), + "lock_arg": format!("{:#x}", lock_arg), "address(deprecated)": address_json(address_payload.clone(), false), "address": address_json(address_payload, true), }); diff --git a/src/subcommands/dao/command.rs b/src/subcommands/dao/command.rs index 4094d9a7..74b1d222 100644 --- a/src/subcommands/dao/command.rs +++ b/src/subcommands/dao/command.rs @@ -6,7 +6,7 @@ use crate::utils::{ AddressParser, ArgParser, CapacityParser, FixedHashParser, FromStrParser, OutPointParser, PrivkeyPathParser, PrivkeyWrapper, }, - other::{get_address, get_network_type}, + other::{get_address, get_network_type, h160_from_slice}, }; use ckb_crypto::secp::SECP256K1; use ckb_sdk::{Address, AddressPayload, HumanCapacity, NetworkType}; @@ -129,8 +129,9 @@ impl TransactArgs { .from_matches_opt(m, "from-account"); result .map(|address_opt| { - address_opt - .map(|address| H160::from_slice(&address.payload().args()).unwrap()) + address_opt.and_then(|address| { + h160_from_slice(address.payload().args().as_ref(), "address").ok() + }) }) .map_err(|_| format!("Invalid value for '--from-account': {}", err)) })? diff --git a/src/subcommands/deploy/mod.rs b/src/subcommands/deploy/mod.rs index ebb2036a..2288e8d6 100644 --- a/src/subcommands/deploy/mod.rs +++ b/src/subcommands/deploy/mod.rs @@ -27,7 +27,7 @@ use crate::utils::{ PrivkeyPathParser, PrivkeyWrapper, }, genesis_info::GenesisInfo, - other::{get_live_cell_with_cache, get_network_type, read_password}, + other::{get_live_cell_with_cache, get_network_type, h160_from_slice, read_password}, rpc::HttpRpcClient, signer::KeyStoreHandlerSigner, tx_helper::{SignerFn, ZERO_HASH}, @@ -309,7 +309,8 @@ impl CliSubCommand for DeploySubCommand<'_> { // Sign if required if m.is_present("sign-now") { - let account = H160::from_slice(from_address.payload().args().as_ref()).unwrap(); + let account = + h160_from_slice(from_address.payload().args().as_ref(), "from-address")?; let signer = { let handler = self.plugin_mgr.keystore_handler(); let change_path = handler.root_key_path(account.clone())?; @@ -380,8 +381,11 @@ impl CliSubCommand for DeploySubCommand<'_> { } let result: Result = parser.parse(input); result - .map(|address| { - H160::from_slice(&address.payload().args()).unwrap() + .and_then(|address| { + h160_from_slice( + address.payload().args().as_ref(), + "address", + ) }) .map_err(|_| err) }) diff --git a/src/subcommands/sudt.rs b/src/subcommands/sudt.rs index 0ebf1895..ba199944 100644 --- a/src/subcommands/sudt.rs +++ b/src/subcommands/sudt.rs @@ -44,7 +44,10 @@ use crate::{ }, cell_dep::{CellDepName, CellDeps}, genesis_info::GenesisInfo, - other::{get_network_type, map_tx_builder_error_2_str, read_password}, + other::{ + get_network_type, h160_from_slice, h160_from_slice_prefix, map_tx_builder_error_2_str, + read_password, + }, rpc::HttpRpcClient, signer::{CommonSigner, KeyStoreHandlerSigner, PrivkeySigner}, }, @@ -244,7 +247,7 @@ impl<'a> SudtSubCommand<'a> { } else { None }; - let owner_account = H160::from_slice(owner.payload().args().as_ref()).unwrap(); + let owner_account = h160_from_slice(owner.payload().args().as_ref(), "owner address")?; let owner_script = Script::from(&owner); let owner_script_hash = owner_script.calc_script_hash(); let receivers = udt_to_vec @@ -368,7 +371,8 @@ impl<'a> SudtSubCommand<'a> { }; let owner_script_hash = Script::from(&owner).calc_script_hash(); - let sender_account = H160::from_slice(&sender.payload().args().as_ref()[0..20]).unwrap(); + let sender_account = + h160_from_slice_prefix(sender.payload().args().as_ref(), "sender address")?; let sender_script = Script::from(&sender); let type_script = udt_type.build_script(&udt_script_id, &owner_script_hash); let cheque_sender_script_hash = Script::new_builder() @@ -426,7 +430,7 @@ impl<'a> SudtSubCommand<'a> { let mut accounts = vec![(format!("sender({})", sender_sighash), sender_account)]; if let Some(addr) = capacity_provider.as_ref() { if *addr != sender { - let account = H160::from_slice(addr.payload().args().as_ref()).unwrap(); + let account = h160_from_slice(addr.payload().args().as_ref(), "address")?; accounts.push((format!("capacity provider({})", addr), account)); } } @@ -549,8 +553,10 @@ impl<'a> SudtSubCommand<'a> { let acp_script_id = get_script_id(&cell_deps, CellDepName::Acp)?; let owner_script_hash = Script::from(&owner).calc_script_hash(); let capacity_provider = capacity_provider.unwrap_or_else(|| to.clone()); - let capacity_provider_account = - H160::from_slice(capacity_provider.payload().args().as_ref()).unwrap(); + let capacity_provider_account = h160_from_slice( + capacity_provider.payload().args().as_ref(), + "capacity provider", + )?; let acp_lock = Script::new_builder() .code_hash(acp_script_id.code_hash.pack()) .hash_type(acp_script_id.hash_type) @@ -703,11 +709,11 @@ impl<'a> SudtSubCommand<'a> { }; let receiver_account = - H160::from_slice(&receiver.payload().args().as_ref()[0..20]).unwrap(); + h160_from_slice_prefix(receiver.payload().args().as_ref(), "receiver address")?; let mut accounts = vec![("receiver".to_string(), receiver_account)]; if let Some(addr) = capacity_provider.as_ref() { if *addr != receiver { - let account = H160::from_slice(addr.payload().args().as_ref()).unwrap(); + let account = h160_from_slice(addr.payload().args().as_ref(), "address")?; accounts.push(("capacity provider".to_string(), account)); } } @@ -818,11 +824,12 @@ impl<'a> SudtSubCommand<'a> { acp_script_id: acp_script_id.clone(), }; - let sender_account = H160::from_slice(&sender.payload().args().as_ref()[0..20]).unwrap(); + let sender_account = + h160_from_slice_prefix(sender.payload().args().as_ref(), "sender address")?; let mut accounts = vec![("sender".to_string(), sender_account)]; if let Some(addr) = capacity_provider.as_ref() { if *addr != receiver { - let account = H160::from_slice(addr.payload().args().as_ref()).unwrap(); + let account = h160_from_slice(addr.payload().args().as_ref(), "address")?; accounts.push(("capacity provider".to_string(), account)); } } diff --git a/src/subcommands/tx.rs b/src/subcommands/tx.rs index f6745836..5dba04bb 100644 --- a/src/subcommands/tx.rs +++ b/src/subcommands/tx.rs @@ -38,7 +38,7 @@ use crate::utils::{ genesis_info::GenesisInfo, other::{ check_capacity, get_genesis_info, get_live_cell, get_live_cell_with_cache, - get_network_type, get_privkey_signer, get_to_data, read_password, + get_network_type, get_privkey_signer, get_to_data, h160_from_slice, read_password, }, rpc::HttpRpcClient, tx_helper::{SignerFn, TxHelper}, @@ -397,10 +397,12 @@ impl CliSubCommand for TxSubCommand<'_> { FromStrParser::::default().from_matches(m, "require-first-n")?; let threshold: u8 = FromStrParser::::default().from_matches(m, "threshold")?; - let sighash_addresses = sighash_addresses + let sighash_addresses: Vec = sighash_addresses .into_iter() - .map(|address| H160::from_slice(address.payload().args().as_ref()).unwrap()) - .collect::>(); + .map(|address| { + h160_from_slice(address.payload().args().as_ref(), "sighash address") + }) + .collect::, _>>()?; let cfg = MultisigConfig::new_with( multisig_script, sighash_addresses, @@ -501,8 +503,11 @@ impl CliSubCommand for TxSubCommand<'_> { .set_network(network) .parse(input); result - .map(|address| { - H160::from_slice(&address.payload().args()).unwrap() + .and_then(|address| { + h160_from_slice( + address.payload().args().as_ref(), + "address", + ) }) .map_err(|_| err) }) @@ -624,10 +629,12 @@ impl CliSubCommand for TxSubCommand<'_> { let since_absolute_epoch_opt: Option = FromStrParser::::default().from_matches_opt(m, "since-absolute-epoch")?; - let sighash_addresses = sighash_addresses + let sighash_addresses: Vec = sighash_addresses .into_iter() - .map(|address| H160::from_slice(address.payload().args().as_ref()).unwrap()) - .collect::>(); + .map(|address| { + h160_from_slice(address.payload().args().as_ref(), "sighash address") + }) + .collect::, _>>()?; let cfg = MultisigConfig::new_with( multisig_script, sighash_addresses, @@ -885,7 +892,7 @@ impl TryFrom for MultisigConfig { .into_iter() .map(|address_string| { Address::from_str(&address_string) - .map(|addr| H160::from_slice(addr.payload().args().as_ref()))? + .map(|addr| h160_from_slice(addr.payload().args().as_ref(), "address"))? .map_err(|err| format!("invalid address: {address_string} error: {err:?}")) }) .collect::, String>>()?; diff --git a/src/subcommands/util.rs b/src/subcommands/util.rs index 1203ddae..4d692ed7 100644 --- a/src/subcommands/util.rs +++ b/src/subcommands/util.rs @@ -36,7 +36,7 @@ use crate::utils::{ PrivkeyPathParser, PrivkeyWrapper, PubkeyHexParser, }, genesis_info::GenesisInfo, - other::{address_json, get_address, get_network_type, read_password}, + other::{address_json, get_address, get_network_type, h160_from_slice, read_password}, rpc::{ChainInfo, HttpRpcClient}, }; use crate::{build_cli, get_version}; @@ -328,7 +328,7 @@ impl CliSubCommand for UtilSubCommand<'_> { Some(pubkey) => AddressPayload::from_pubkey(&pubkey), None => get_address(None, m)?, }; - let lock_arg = H160::from_slice(address_payload.args().as_ref()).unwrap(); + let lock_arg = h160_from_slice(address_payload.args().as_ref(), "address payload")?; let old_address = OldAddress::new_default(lock_arg.clone()); eprintln!( @@ -369,8 +369,9 @@ message = "0x" AddressParser::new_sighash().from_matches_opt(m, "from-account"); result .map(|address_opt| { - address_opt.map(|address| { - H160::from_slice(&address.payload().args()).unwrap() + address_opt.and_then(|address| { + h160_from_slice(address.payload().args().as_ref(), "address") + .ok() }) }) .map_err(|_| err) @@ -444,8 +445,9 @@ message = "0x" AddressParser::new_sighash().from_matches_opt(m, "from-account"); result .map(|address_opt| { - address_opt.map(|address| { - H160::from_slice(&address.payload().args()).unwrap() + address_opt.and_then(|address| { + h160_from_slice(address.payload().args().as_ref(), "address") + .ok() }) }) .map_err(|_| err) @@ -505,8 +507,9 @@ message = "0x" AddressParser::new_sighash().from_matches_opt(m, "from-account"); result .map(|address_opt| { - address_opt.map(|address| { - H160::from_slice(&address.payload().args()).unwrap() + address_opt.and_then(|address| { + h160_from_slice(address.payload().args().as_ref(), "address") + .ok() }) }) .map_err(|_| err) @@ -858,8 +861,10 @@ fn search_path( extended_address: Address, password: Option, ) -> Result { - let target = H160::from_slice(extended_address.payload().args().as_ref()) - .map_err(|err| format!("parse extended address lock args error: {}", err))?; + let target = h160_from_slice( + extended_address.payload().args().as_ref(), + "extended address", + )?; let key_set = plugin_mgr .keystore_handler() .derived_key_set_by_index(hash160, 0, 2000, 0, 2000, password)?; diff --git a/src/subcommands/wallet.rs b/src/subcommands/wallet.rs index 58381bf4..6b5f71aa 100644 --- a/src/subcommands/wallet.rs +++ b/src/subcommands/wallet.rs @@ -46,7 +46,7 @@ use crate::utils::{ genesis_info::GenesisInfo, other::{ check_capacity, get_address, get_arg_value, get_genesis_info, get_network_type, - get_to_data, map_tx_builder_error_2_str, read_password, to_live_cell_info, + get_to_data, h160_from_slice, map_tx_builder_error_2_str, read_password, to_live_cell_info, }, rpc::HttpRpcClient, signer::KeyStoreHandlerSigner, @@ -162,7 +162,9 @@ impl<'a> WalletSubCommand<'a> { .set_network(network_type) .parse(&input); result - .map(|address| H160::from_slice(&address.payload().args()).unwrap()) + .and_then(|address| { + h160_from_slice(address.payload().args().as_ref(), "address") + }) .map_err(|_| err) }) }) @@ -272,13 +274,16 @@ impl<'a> WalletSubCommand<'a> { SinceSource::default(), )]; - let from_lock_arg = H160::from_slice(from_address.payload().args().as_ref()).unwrap(); + let from_lock_arg = + h160_from_slice(from_address.payload().args().as_ref(), "from-address")?; let mut path_map: HashMap = Default::default(); let (change_address_payload, change_path) = if let Some(last_change_address) = last_change_address_opt.as_ref() { // Behave like HD wallet - let change_last = - H160::from_slice(last_change_address.payload().args().as_ref()).unwrap(); + let change_last = h160_from_slice( + last_change_address.payload().args().as_ref(), + "change-address", + )?; let key_set = self.plugin_mgr.keystore_handler().derived_key_set( from_lock_arg.clone(), receiving_address_length, @@ -326,7 +331,8 @@ impl<'a> WalletSubCommand<'a> { } if let Some(last_change_address) = last_change_address_opt.as_ref() { let change_last = - H160::from_slice(last_change_address.payload().args().as_ref()).unwrap(); + H160::from_slice(last_change_address.payload().args().as_ref()) + .map_err(|err| format!("invalid H160 from change-address: {}", err))?; signer.cache_key_set( from_lock_arg.clone(), receiving_address_length, @@ -624,7 +630,8 @@ impl CliSubCommand for WalletSubCommand<'_> { }; let mut lock_scripts = vec![Script::from(&address_payload)]; if m.is_present("derived") { - let lock_arg = H160::from_slice(address_payload.args().as_ref()).unwrap(); + let lock_arg = + h160_from_slice(address_payload.args().as_ref(), "address payload")?; let key_set = self .plugin_mgr diff --git a/src/utils/other.rs b/src/utils/other.rs index 044da459..9038b148 100644 --- a/src/utils/other.rs +++ b/src/utils/other.rs @@ -383,3 +383,16 @@ pub(crate) fn map_tx_builder_error_2_str(no_max_tx_fee: bool, err: TxBuilderErro } err.to_string() } + +/// Convert a byte slice to H160 with a context-specific error message. +pub(crate) fn h160_from_slice(data: &[u8], context: &str) -> Result { + H160::from_slice(data).map_err(|e| format!("invalid H160 from {}: {}", context, e)) +} + +/// Take the first 20 bytes of a slice and convert to H160 with context error. +pub(crate) fn h160_from_slice_prefix(data: &[u8], context: &str) -> Result { + let bytes = data + .get(0..20) + .ok_or_else(|| format!("{} payload too short for H160", context))?; + H160::from_slice(bytes).map_err(|e| format!("invalid H160 from {}: {}", context, e)) +} diff --git a/src/utils/signer.rs b/src/utils/signer.rs index 19b6282d..0aaa36e3 100644 --- a/src/utils/signer.rs +++ b/src/utils/signer.rs @@ -77,7 +77,9 @@ impl PrivkeySigner { pub fn add_privkey(&mut self, privkey: PrivkeyWrapper) { let pubkey = secp256k1::PublicKey::from_secret_key(&SECP256K1, &privkey); - let id = H160::from_slice(&blake2b_256(&pubkey.serialize()[..])[0..20]).unwrap(); + // Safe: blake2b_256 always produces 32 bytes, [0..20] is always exactly 20 bytes + let id = H160::from_slice(&blake2b_256(&pubkey.serialize()[..])[0..20]) + .expect("H160::from_slice on 20-byte hash output should never fail"); self.privkeys.insert(id.clone(), privkey); self.ids.insert(id.clone(), id); } @@ -90,7 +92,9 @@ impl PrivkeySigner { .args(Bytes::from(account.as_bytes().to_vec()).pack()) .build() .calc_script_hash(); - let lock_hash160 = H160::from_slice(&script_hash.as_slice()[0..20]).unwrap(); + // Safe: script_hash is always 32 bytes, [0..20] is always exactly 20 bytes + let lock_hash160 = H160::from_slice(&script_hash.as_slice()[0..20]) + .expect("H160::from_slice on 20-byte script hash should never fail"); self.ids.insert(lock_hash160, account); true } else { @@ -104,7 +108,10 @@ impl Signer for PrivkeySigner { if id.len() != 20 { return false; } - self.ids.contains_key(&H160::from_slice(id).unwrap()) + // Safe: guarded by id.len() != 20 check above + self.ids.contains_key( + &H160::from_slice(id).expect("H160::from_slice on 20-byte id should never fail"), + ) } fn sign( @@ -117,7 +124,8 @@ impl Signer for PrivkeySigner { if id.len() != 20 { return Err(SignerError::IdNotFound); } - let hash160 = H160::from_slice(id).unwrap(); + // Safe: guarded by id.len() != 20 check above + let hash160 = H160::from_slice(id).map_err(|_| SignerError::IdNotFound)?; let account = self.ids.get(&hash160).ok_or(SignerError::IdNotFound)?; let privkey = self.privkeys.get(account).expect("no privkey found"); privkey.sign(id, message, recoverable, tx) @@ -165,7 +173,9 @@ impl KeyStoreHandlerSigner { .args(Bytes::from(account.as_bytes().to_vec()).pack()) .build() .calc_script_hash(); - let lock_hash160 = H160::from_slice(&script_hash.as_slice()[0..20]).unwrap(); + // Safe: script_hash is always 32 bytes, [0..20] is always exactly 20 bytes + let lock_hash160 = H160::from_slice(&script_hash.as_slice()[0..20]) + .expect("H160::from_slice on 20-byte script hash should never fail"); self.ids .insert(lock_hash160, (DerivationPath::default(), None, account)); true diff --git a/src/utils/tx_helper.rs b/src/utils/tx_helper.rs index eeb2c14f..3aa20d9d 100644 --- a/src/utils/tx_helper.rs +++ b/src/utils/tx_helper.rs @@ -17,6 +17,7 @@ use ckb_sdk::constants::{MultisigScript, SECP_SIGNATURE_SIZE, SIGHASH_TYPE_HASH} use ckb_sdk::{unlock::MultisigConfig, Since}; use crate::utils::genesis_info::GenesisInfo; +use crate::utils::other::{h160_from_slice, h160_from_slice_prefix}; // TODO: Add dao support @@ -198,7 +199,7 @@ impl TxHelper { ] .contains(&code_hash) { - let hash160 = H160::from_slice(&lock_arg[..20]).unwrap(); + let hash160 = h160_from_slice_prefix(&lock_arg, "lock_arg")?; if !self.multisig_configs.contains_key(&hash160) { return Err(format!( "No mutisig config found for input(no.{}) lock_arg prefix: {:#x}", @@ -260,7 +261,7 @@ impl TxHelper { continue; } - let multisig_hash160 = H160::from_slice(&lock_arg[..20]).unwrap(); + let multisig_hash160 = h160_from_slice_prefix(&lock_arg, "lock_arg")?; let lock_args = if [ MultisigScript::Legacy.script_id().code_hash.pack(), MultisigScript::V2.script_id().code_hash.pack(), @@ -273,7 +274,7 @@ impl TxHelper { .clone() } else { let mut lock_args = HashSet::default(); - lock_args.insert(H160::from_slice(lock_arg.as_ref()).unwrap()); + lock_args.insert(h160_from_slice(lock_arg.as_ref(), "lock_arg")?); lock_args }; if signer(&lock_args, &h256!("0x0"), &Transaction::default().into())?.is_some() { @@ -324,7 +325,7 @@ impl TxHelper { ] .contains(&code_hash) { - let hash160 = H160::from_slice(&lock_arg[..20]).unwrap(); + let hash160 = h160_from_slice_prefix(&lock_arg, "lock_arg")?; let multisig_config = self.multisig_configs.get(&hash160).unwrap(); let threshold = multisig_config.threshold() as usize; let mut data = BytesMut::from(&multisig_config.to_witness_data()[..]);