fix: validate force_small_change_as_fee in the API server

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

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
 

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<H256> {
-        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"));
+    }
+}

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::<u64>::default().parse(&fee_rate)?;
-        let force_small_change_as_fee: Option<u64> =
-            force_small_change_as_fee.map(|s| CapacityParser.parse(&s).unwrap().into());
+        let force_small_change_as_fee: Option<u64> = 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::<u32>::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);
                             }