fix contract view direction

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 {
+	let suggested = format_net_change(view.suggested_net_change);
-		Some(v) => amount_to_hr_string(v.unsigned_abs(), false),
+	table.add_row(row![bFC->"Suggested Counterparty Net Change", bGC->suggested]);
-		None => String::from("None"),
+	let agreed = format_net_change(view.agreed_net_change);
-	};
-	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"),
-	};
 	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<i64>) -> 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");
+	}
+}

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.

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<Option<i64>, 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<C, K>(
 	w: &mut WalletBackend<C, K>,
@@ -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 = suggested_net_change(&slate.state, slate.amount)?;
-	let suggested_net_change: Option<i64> = 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,
-	};
 	// 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
+		);
+	}
+}