diff --git a/CHANGELOG.md b/CHANGELOG.md index 6366398d..93a972d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,6 @@ # Unreleased +* Fix: API server `transfer` rejects an invalid `force_small_change_as_fee` +* Fix: `wallet transfer` saturates the capacity in its fee suggestion instead of overflowing * `rpc get_live_cell` now returns `block_hash` (upgrade `ckb-jsonrpc-types` to 1.2) * Update `ckb-types` and related ckb crates to the 1.1/1.2 wave * Update `ckb-sdk` to 5.1 diff --git a/docs/API-Server.md b/docs/API-Server.md index bab5845d..0622f212 100644 --- a/docs/API-Server.md +++ b/docs/API-Server.md @@ -49,6 +49,7 @@ TransferArgs fields: to_address - Target address from_locked_address - (optional) The time locked multisig address to search live cells to_data - (optional) Hex data store in target cell + force_small_change_as_fee - (optional) When there is no more inputs to create a change cell, force the remaining capacity as fee (unit: CKB, passed as a string, example: "0.001") #### Examples diff --git a/src/subcommands/api_server.rs b/src/subcommands/api_server.rs index 2f02bc63..3530a458 100644 --- a/src/subcommands/api_server.rs +++ b/src/subcommands/api_server.rs @@ -18,7 +18,9 @@ use super::{CliSubCommand, Output, TransferArgs, WalletSubCommand}; use crate::plugin::PluginManager; use crate::utils::{ arg, - arg_parser::{AddressParser, ArgParser, FromStrParser, PrivkeyPathParser, PrivkeyWrapper}, + arg_parser::{ + AddressParser, ArgParser, CapacityParser, FromStrParser, PrivkeyPathParser, PrivkeyWrapper, + }, genesis_info::GenesisInfo, other::{get_genesis_info, get_network_type}, rpc::HttpRpcClient, @@ -217,7 +219,8 @@ impl ApiRpcImpl { impl ApiRpc for ApiRpcImpl { fn transfer(&self, args: HttpTransferArgs) -> RpcResult { - log::info!("[call]: tranfer({:?})", args); + log::info!("[call]: transfer({:?})", args); + validate_force_small_change_as_fee(args.force_small_change_as_fee.as_deref())?; if let Some(privkey_path) = self.privkey_path.clone() { self.with_wallet(|cmd| { cmd.transfer(args.into_full_args(privkey_path), false) @@ -374,6 +377,17 @@ fn internal_err(message: String) -> RpcError { } } +/// Validate the client-supplied `force_small_change_as_fee` before any wallet lock is +/// taken, so a value that cannot be parsed as a capacity is reported as invalid params. +fn validate_force_small_change_as_fee(input: Option<&str>) -> RpcResult<()> { + if let Some(fee) = input { + CapacityParser + .parse(fee) + .map_err(RpcError::invalid_params)?; + } + Ok(()) +} + #[derive(Clone, Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct HttpTransferArgs { @@ -413,3 +427,38 @@ pub struct GetCapacityResponse { pub immature: u64, pub dao: u64, } + +#[cfg(test)] +mod tests { + use super::{validate_force_small_change_as_fee, HttpTransferArgs, RpcErrorCode}; + + fn transfer_args(force_small_change_as_fee: serde_json::Value) -> serde_json::Value { + serde_json::json!({ + "capacity": 10000000000u64, + "fee_rate": 1000u64, + "to_address": "ckt1qyqdfjzl8ju2vfwjtl4mttx6me09hayzfldq8m3a0y", + "force_small_change_as_fee": force_small_change_as_fee, + }) + } + + #[test] + fn reject_malformed_force_small_change_as_fee() { + let err = validate_force_small_change_as_fee(Some("NOT_A_CAPACITY")).unwrap_err(); + assert_eq!(err.code, RpcErrorCode::InvalidParams); + } + + #[test] + fn accept_valid_or_absent_force_small_change_as_fee() { + assert!(validate_force_small_change_as_fee(Some("0.001")).is_ok()); + assert!(validate_force_small_change_as_fee(None).is_ok()); + } + + #[test] + fn force_small_change_as_fee_stays_a_capacity_string() { + let args = transfer_args(serde_json::json!("0.001")); + let parsed: HttpTransferArgs = serde_json::from_value(args).expect("valid transfer args"); + assert_eq!(parsed.force_small_change_as_fee.as_deref(), Some("0.001")); + let full = parsed.into_full_args("/tmp/privkey".to_string()); + assert_eq!(full.force_small_change_as_fee.as_deref(), Some("0.001")); + } +} diff --git a/src/subcommands/wallet.rs b/src/subcommands/wallet.rs index 6b5f71aa..7fea9696 100644 --- a/src/subcommands/wallet.rs +++ b/src/subcommands/wallet.rs @@ -181,8 +181,9 @@ impl<'a> WalletSubCommand<'a> { .transpose()?; let to_capacity: u64 = CapacityParser.parse(&capacity)?.into(); let fee_rate: u64 = FromStrParser::::default().parse(&fee_rate)?; - let force_small_change_as_fee: Option = - force_small_change_as_fee.map(|s| CapacityParser.parse(&s).unwrap().into()); + let force_small_change_as_fee: Option = force_small_change_as_fee + .map(|s| CapacityParser.parse(&s).map(Into::into)) + .transpose()?; let receiving_address_length: u32 = derive_receiving_address_length .map(|input| FromStrParser::::default().parse(&input)) .transpose()? @@ -463,7 +464,8 @@ impl<'a> WalletSubCommand<'a> { if msg.contains(prefix) { let left_capacity = HumanCapacity::from_str(&msg[prefix.len()..]); if let Ok(left_capacity) = left_capacity { - let suggest_capacity = HumanCapacity(left_capacity.0 + to_capacity); + let suggest_capacity = + HumanCapacity(left_capacity.0.saturating_add(to_capacity)); return format!("{}, try to transfer {} or try parameter `--max-tx-fee` to make small left capacity as transaction fee", err, suggest_capacity); }