diff --git a/controller/src/display.rs b/controller/src/display.rs index 05e2ce0..203255b 100644 --- a/controller/src/display.rs +++ b/controller/src/display.rs @@ -646,15 +646,9 @@ pub fn contract_view(slate: &Slate, view: &ContractView) { table.add_row(row![bFC->"State", bGC->slate.state]); table.add_row(row![bFC->"Participants", bGC->view.num_participants]); table.add_row(row![bFC->"Signatures", bGC->view.num_sigs]); - let suggested = match view.suggested_net_change { - Some(v) => amount_to_hr_string(v.unsigned_abs(), false), - None => String::from("None"), - }; - table.add_row(row![bFC->"Suggested Net Change", bGC->suggested]); - let agreed = match view.agreed_net_change { - Some(v) => amount_to_hr_string(v.unsigned_abs(), false), - None => String::from("None"), - }; + let suggested = format_net_change(view.suggested_net_change); + table.add_row(row![bFC->"Suggested Counterparty Net Change", bGC->suggested]); + let agreed = format_net_change(view.agreed_net_change); table.add_row(row![bFC->"Agreed Net Change", bGC->agreed]); table.add_row(row![bFC->"Executed", bGC->view.is_executed]); @@ -662,3 +656,26 @@ pub fn contract_view(slate: &Slate, view: &ContractView) { table.printstd(); println!(); } + +fn format_net_change(change: Option) -> String { + match change { + Some(value) => format!( + "{}{}", + if value < 0 { "-" } else { "+" }, + amount_to_hr_string(value.unsigned_abs(), false) + ), + None => String::from("None"), + } +} + +#[cfg(test)] +mod tests { + use super::format_net_change; + + #[test] + fn net_change_sign() { + assert_eq!(format_net_change(Some(1_000_000_000)), "+1.000000000"); + assert_eq!(format_net_change(Some(-1_000_000_000)), "-1.000000000"); + assert_eq!(format_net_change(None), "None"); + } +} diff --git a/libwallet/src/contract/actions/README.md b/libwallet/src/contract/actions/README.md index ee71b7c..f398984 100644 --- a/libwallet/src/contract/actions/README.md +++ b/libwallet/src/contract/actions/README.md @@ -54,7 +54,6 @@ Separating side effects until the 'save_step' part would make these functions mu - Let `view` report whether a slatepack was encrypted for this wallet. - Show all inputs, outputs, fees and resulting balance changes before the CLI signs. - Distinguish the transfer amount from the fee-adjusted balance change in `view`. - - Correct the suggested change direction in `view` and keep that direction in the CLI. - Raise the replacement fee when `revoke` races a transaction already in the mempool. - Merge invoice-proof retrieval with the existing payment-proof API. - Confirm that invoice-proof retrieval excludes the transaction fee from the amount. diff --git a/libwallet/src/contract/actions/view.rs b/libwallet/src/contract/actions/view.rs index 02049ce..bb7edf0 100644 --- a/libwallet/src/contract/actions/view.rs +++ b/libwallet/src/contract/actions/view.rs @@ -23,6 +23,17 @@ use crate::internal::updater; use crate::slate::{Slate, SlateState}; use crate::types::NodeClient; +fn suggested_net_change(state: &SlateState, amount: u64) -> Result, Error> { + let sign = match state { + SlateState::Standard1 => 1, + SlateState::Invoice1 => -1, + _ => return Ok(None), + }; + let amount = i64::try_from(amount) + .map_err(|_| Error::GenericError(format!("Slate amount {} exceeds i64", amount)))?; + Ok(Some(sign * amount)) +} + /// View contract pub fn view( w: &mut WalletBackend, @@ -52,16 +63,7 @@ where slate.num_participants ))); } - // Checked conversion so an out-of-range amount can't wrap into a bogus net change. - let suggested_net_change: Option = match slate.state { - SlateState::Invoice1 => Some(i64::try_from(slate.amount).map_err(|_| { - Error::GenericError(format!("Slate amount {} exceeds i64", slate.amount)) - })?), - SlateState::Standard1 => Some(-i64::try_from(slate.amount).map_err(|_| { - Error::GenericError(format!("Slate amount {} exceeds i64", slate.amount)) - })?), - _ => None, - }; + let suggested_net_change = suggested_net_change(&slate.state, slate.amount)?; // A contract is executed once the transaction it produced has confirmed. That is // recorded on our own tx log entry for this slate, so no chain lookup is needed; a // slate we have never signed simply has no entry and is not executed. @@ -95,3 +97,24 @@ where }; Ok(ct_view) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn suggested_change_direction() { + assert_eq!( + suggested_net_change(&SlateState::Standard1, 10).unwrap(), + Some(10) + ); + assert_eq!( + suggested_net_change(&SlateState::Invoice1, 10).unwrap(), + Some(-10) + ); + assert_eq!( + suggested_net_change(&SlateState::Standard2, 10).unwrap(), + None + ); + } +}