Merge branch 'gakonst-asset-scale' into gakonst-precision-loss

Author: gakonst

crates/interledger-service-util/src/exchange_rates_service.rs

@@ -90,17 +90,52 @@ where
                 .build()));
             };
 
-            // Note that this is not an overflow safe operation. Given realistic
-            // assumptions, the asset scale will be <=18 and >=1.
-            // It is doubtful that the exchange rate between two assets,
-            // multiplied by 18 would exceed std::f64::MAX
-            let scaled_rate = rate.normalize_scale(ConvertDetails {
+            // Can we overflow here?
+            let outgoing_amount = (request.prepare.amount() as f64) * rate;
+            let outgoing_amount = outgoing_amount.normalize_scale(ConvertDetails {
                 from: request.from.asset_scale(),
                 to: request.to.asset_scale(),
             });
-            let outgoing_amount = (request.prepare.amount() as f64) * scaled_rate;
-            request.prepare.set_amount(outgoing_amount as u64);
-            trace!("Converted incoming amount of: {} {} (scale {}) from account {} to outgoing amount of: {} {} (scale {}) for account {}", request.original_amount, request.from.asset_code(), request.from.asset_scale(), request.from.id(), outgoing_amount, request.to.asset_code(), request.to.asset_scale(), request.to.id());
+
+            match outgoing_amount {
+                Ok(outgoing_amount) => {
+                    // f64 which cannot fit in u64 gets cast as 0
+                    if outgoing_amount != 0.0 && outgoing_amount as u64 == 0 {
+                        return Box::new(err(RejectBuilder {
+                            code: ErrorCode::F08_AMOUNT_TOO_LARGE,
+                            message: format!(
+                                "Could not cast outgoing amount to u64 {}",
+                                outgoing_amount,
+                            )
+                            .as_bytes(),
+                            triggered_by: Some(&self.ilp_address),
+                            data: &[],
+                        }
+                        .build()));
+                    }
+                    request.prepare.set_amount(outgoing_amount as u64);
+                    trace!("Converted incoming amount of: {} {} (scale {}) from account {} to outgoing amount of: {} {} (scale {}) for account {}",
+                        request.original_amount, request.from.asset_code(), request.from.asset_scale(), request.from.id(),
+                        outgoing_amount, request.to.asset_code(), request.to.asset_scale(), request.to.id());
+                }
+                Err(_) => {
+                    return Box::new(err(RejectBuilder {
+                        code: ErrorCode::F08_AMOUNT_TOO_LARGE,
+                        message: format!(
+                            "Could not convert exchange rate from {}:{} to: {}:{}. Got incoming amount: {}",
+                            request.from.asset_code(),
+                            request.from.asset_scale(),
+                            request.to.asset_code(),
+                            request.to.asset_scale(),
+                            request.prepare.amount(),
+                        )
+                        .as_bytes(),
+                        triggered_by: Some(&self.ilp_address),
+                        data: &[],
+                    }
+                    .build()));
+                }
+            }
         }
 
         Box::new(self.next.send_request(request))

crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs

@@ -820,7 +820,7 @@ where
         let engine_scale = self.asset_scale;
         Box::new(
             result(BigUint::from_str(&body.amount).map_err(move |err| {
-                let error_msg = format!("Error converting to U256 {:?}", err);
+                let error_msg = format!("Error converting to BigUint {:?}", err);
                 error!("{:?}", error_msg);
                 (StatusCode::from_u16(400).unwrap(), error_msg)
             }))
@@ -832,13 +832,21 @@ where
                     to: engine_scale,
                 });
 
-                // Typecast from num_bigint::BigUInt because we're using
-                // ethereum_types::U256 for all rust-web3 related functionality
-                result(U256::from_dec_str(&amount.to_string()).map_err(move |err| {
-                    let error_msg = format!("Error converting to U256 {:?}", err);
-                    error!("{:?}", error_msg);
-                    (StatusCode::from_u16(400).unwrap(), error_msg)
-                }))
+                result(amount)
+                    .map_err(move |err| {
+                        let error_msg = format!("Error scaling amount: {:?}", err);
+                        error!("{:?}", error_msg);
+                        (StatusCode::from_u16(400).unwrap(), error_msg)
+                    })
+                    .and_then(move |amount| {
+                        // Typecast from num_bigint::BigUInt because we're using
+                        // ethereum_types::U256 for all rust-web3 related functionality
+                        result(U256::from_dec_str(&amount.to_string()).map_err(move |err| {
+                            let error_msg = format!("Error converting to U256 {:?}", err);
+                            error!("{:?}", error_msg);
+                            (StatusCode::from_u16(400).unwrap(), error_msg)
+                        }))
+                    })
             })
             .and_then(move |amount| {
                 self_clone

crates/interledger-settlement/src/api.rs

@@ -191,22 +191,30 @@ impl_web! {
                         from: engine_scale,
                         to: account.asset_scale(),
                     });
-                    // If we'd overflow, settle for the maximum u64 value
-                    let safe_amount = if let Some(amount_u64) = amount.to_u64() {
-                        amount_u64
-                    } else {
-                        debug!("Amount settled from engine overflowed during conversion to connector scale: {}. Settling for u64::MAX", amount);
-                        std::u64::MAX
-                    };
-                    store.update_balance_for_incoming_settlement(account_id, safe_amount, idempotency_key)
+                    result(amount.clone())
                     .map_err(move |_| {
-                        let error_msg = format!("Error updating balance of account: {} for incoming settlement of amount: {}", account_id, amount);
+                        let error_msg = format!("Could not convert amount: {:?}", amount);
                         error!("{}", error_msg);
                         (StatusCode::from_u16(500).unwrap(), error_msg)
                     })
-                    .and_then(move |_| {
-                        let ret = json!(Quantity::new(safe_amount, account.asset_scale()));
-                        Ok((StatusCode::OK, ret.to_string().into()))
+                    .and_then(move |amount| {
+                        // If we'd overflow, settle for the maximum u64 value
+                        let amount = if let Some(amount_u64) = amount.to_u64() {
+                            amount_u64
+                        } else {
+                            debug!("Amount settled from engine overflowed during conversion to connector scale: {:?}. Settling for u64::MAX", amount);
+                            std::u64::MAX
+                        };
+                        store.update_balance_for_incoming_settlement(account_id, amount, idempotency_key)
+                        .map_err(move |_| {
+                            let error_msg = format!("Error updating balance of account: {} for incoming settlement of amount: {}", account_id, amount);
+                            error!("{}", error_msg);
+                            (StatusCode::from_u16(500).unwrap(), error_msg)
+                        })
+                        .and_then(move |_| {
+                            let ret = json!(Quantity::new(amount, account.asset_scale()));
+                            Ok((StatusCode::OK, ret.to_string().into()))
+                        })
                     })
                 })
             }))
@@ -346,7 +354,7 @@ mod tests {
             let q: Quantity = serde_json::from_slice(ret.body()).unwrap();
             assert_eq!(ret.status(), 200);
             assert_eq!(q.amount, 1.to_string());
-            let leftovers = 123 - 1.normalize_scale(ConvertDetails { from: 9, to: 11 });
+            let leftovers = 123 - 1u64.normalize_scale(ConvertDetails { from: 9, to: 11 }).unwrap();
             assert_eq!(leftovers, 23);
             assert_eq!(q.scale, 9);
         }