verify contract slate commitments

controller/src/display.rs

@@ -15,7 +15,7 @@
 use crate::core::core::FeeFields;
 use crate::core::core::{self, amount_to_hr_string};
 use crate::core::global;
-use crate::libwallet::contract::types::ContractView;
+use crate::libwallet::contract::types::{ContractView, OwnCommitmentStatus};
 use crate::libwallet::{
 	AcctPathMapping, Error, OutputCommitMapping, OutputStatus, Slate, TxLogEntry, ViewWallet,
 	WalletInfo,
@@ -651,6 +651,18 @@ pub fn contract_view(slate: &Slate, view: &ContractView) {
 	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]);
+	let (unexpected, warning) = match view.own_commitment_status {
+		OwnCommitmentStatus::UnexpectedInput => ("Input", true),
+		OwnCommitmentStatus::UnexpectedOutput => ("Output", true),
+		OwnCommitmentStatus::UnexpectedInputAndOutput => ("Input and output", true),
+		OwnCommitmentStatus::Clean => ("No", false),
+		OwnCommitmentStatus::Unknown => ("Unknown", false),
+	};
+	if warning {
+		table.add_row(row![bFC->"Unexpected Own Commitments", bFR->unexpected]);
+	} else {
+		table.add_row(row![bFC->"Unexpected Own Commitments", bGC->unexpected]);
+	}
 
 	table.set_format(*prettytable::format::consts::FORMAT_NO_BORDER_LINE_SEPARATOR);
 	table.printstd();

controller/tests/contract_early_lock.rs

@@ -20,8 +20,9 @@ extern crate log;
 use grin_wallet_libwallet as libwallet;
 
 use grin_core::consensus;
+use grin_core::core::transaction::{Input, OutputFeatures};
 use libwallet::contract::my_fee_contribution;
-use libwallet::contract::types::{ContractNewArgsAPI, ContractSetupArgsAPI};
+use libwallet::contract::types::{ContractNewArgsAPI, ContractSetupArgsAPI, OwnCommitmentStatus};
 use libwallet::{OutputStatus, Slate, SlateState};
 use std::sync::atomic::Ordering;
 use std::thread;
@@ -34,11 +35,13 @@ use std::path::PathBuf;
 
 /// contract new with --add-outputs
 fn contract_early_lock_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> {
-	// create a single wallet and mine 5 blocks
+	// create two wallets and mine 5 blocks in each
 	let (wallets, _chain, stopper, _bh) =
-		create_wallets(vec![vec![("default", 5)]], test_dir).unwrap();
+		create_wallets(vec![vec![("default", 5)], vec![("default", 5)]], test_dir).unwrap();
 	let send_wallet = wallets[0].0.clone();
 	let send_mask = wallets[0].1.as_ref();
+	let recv_wallet = wallets[1].0.clone();
+	let recv_mask = wallets[1].1.as_ref();
 
 	// Confirm all our outputs are unspent
 	wallet::controller::owner_single_use(
@@ -102,6 +105,79 @@ fn contract_early_lock_tx_impl(test_dir: &'static str) -> Result<(), libwallet::
 		},
 	)?;
 
+	// The receiver signs without seeing the sender's locked commitments.
+	wallet::controller::owner_single_use(
+		recv_wallet.clone(),
+		recv_mask,
+		PathBuf::from(test_dir),
+		|api, m| {
+			slate = api.contract_sign(
+				m,
+				&slate,
+				&ContractSetupArgsAPI {
+					net_change: Some(80_000_000_000),
+					..Default::default()
+				},
+			)?;
+			Ok(())
+		},
+	)?;
+	assert_eq!(slate.state, SlateState::Standard2);
+
+	// Even a commitment reserved for this contract must not be present before we add it.
+	wallet::controller::owner_single_use(
+		send_wallet.clone(),
+		send_mask,
+		PathBuf::from(test_dir),
+		|api, m| {
+			let (_, outputs) = api.retrieve_outputs(m, false, false, None)?;
+			let output = outputs
+				.iter()
+				.find(|output| output.output.status == OutputStatus::Locked)
+				.expect("sender has a locked contract input");
+			let features = if output.output.is_coinbase {
+				OutputFeatures::Coinbase
+			} else {
+				OutputFeatures::Plain
+			};
+			let mut changed = slate.clone();
+			changed.tx = Some(
+				changed
+					.tx_or_err()?
+					.clone()
+					.with_input(Input::new(features, output.commit)),
+			);
+			let view = api.contract_view(m, &changed)?;
+			assert_eq!(
+				view.own_commitment_status,
+				OwnCommitmentStatus::UnexpectedInput
+			);
+			let err = api
+				.contract_sign(m, &changed, &ContractSetupArgsAPI::default())
+				.unwrap_err();
+			match err {
+				libwallet::Error::GenericError(message) => assert_eq!(
+					message,
+					"Contract slate contains an unexpected input commitment from this wallet"
+				),
+				err => panic!("unexpected error: {}", err),
+			}
+			Ok(())
+		},
+	)?;
+
+	// The sender adds the reserved commitments and completes the contract.
+	wallet::controller::owner_single_use(
+		send_wallet.clone(),
+		send_mask,
+		PathBuf::from(test_dir),
+		|api, m| {
+			slate = api.contract_sign(m, &slate, &ContractSetupArgsAPI::default())?;
+			Ok(())
+		},
+	)?;
+	assert_eq!(slate.state, SlateState::Standard3);
+
 	// let logging finish
 	stopper.store(false, Ordering::Relaxed);
 	thread::sleep(Duration::from_millis(200));

controller/tests/contract_srs.rs

@@ -19,9 +19,10 @@ extern crate log;
 
 use grin_wallet_libwallet as libwallet;
 
+use grin_core::core::transaction::{CommitWrapper, Input, Inputs, OutputFeatures, Transaction};
 use impls::test_framework::{self};
 use libwallet::contract::my_fee_contribution;
-use libwallet::contract::types::{ContractNewArgsAPI, ContractSetupArgsAPI};
+use libwallet::contract::types::{ContractNewArgsAPI, ContractSetupArgsAPI, OwnCommitmentStatus};
 use libwallet::{Slate, SlateState, TxLogEntryType};
 use std::sync::atomic::Ordering;
 use std::thread;
@@ -78,6 +79,171 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error>
 		},
 	)?;
 	assert_eq!(slate.state, SlateState::Standard2);
+	wallet::controller::owner_single_use(
+		recv_wallet.clone(),
+		recv_mask,
+		PathBuf::from(test_dir),
+		|api, m| {
+			let view = api.contract_view(m, &slate)?;
+			assert_eq!(view.own_commitment_status, OwnCommitmentStatus::Clean);
+			Ok(())
+		},
+	)?;
+	wallet::controller::owner_single_use(
+		send_wallet.clone(),
+		send_mask,
+		PathBuf::from(test_dir),
+		|api, m| {
+			let view = api.contract_view(m, &slate)?;
+			assert_eq!(view.own_commitment_status, OwnCommitmentStatus::Clean);
+			let mut no_tx = slate.clone();
+			no_tx.tx = None;
+			let view = api.contract_view(m, &no_tx)?;
+			assert_eq!(view.own_commitment_status, OwnCommitmentStatus::Unknown);
+			Ok(())
+		},
+	)?;
+
+	// The sender must reject an unused commitment from its wallet if it was inserted by the receiver.
+	wallet::controller::owner_single_use(
+		send_wallet.clone(),
+		send_mask,
+		PathBuf::from(test_dir),
+		|api, m| {
+			let (_, outputs) = api.retrieve_outputs(m, false, false, None)?;
+			let tx = slate.tx_or_err()?;
+			let input_commits = Vec::<CommitWrapper>::from(&tx.inputs());
+			let child_index = {
+				let mut wallet = api.wallet_inst.lock();
+				let wallet = wallet.lc_provider()?.wallet_inst()?;
+				let parent = wallet.parent_key_id();
+				wallet.current_child_index(&parent)?
+			};
+			let mut changed = slate.clone();
+			changed.tx = Some(
+				Transaction::new(
+					Inputs::CommitOnly(input_commits.clone()),
+					tx.outputs(),
+					tx.kernels(),
+				)
+				.with_offset(tx.offset.clone()),
+			);
+			let err = api
+				.contract_sign(m, &changed, &ContractSetupArgsAPI::default())
+				.unwrap_err();
+			match err {
+				libwallet::Error::GenericError(message) => {
+					assert_eq!(message, "Contract slate input features are missing")
+				}
+				err => panic!("unexpected error: {}", err),
+			}
+			let view = api.contract_view(m, &changed)?;
+			assert_eq!(view.own_commitment_status, OwnCommitmentStatus::Unknown);
+			let current_child_index = {
+				let mut wallet = api.wallet_inst.lock();
+				let wallet = wallet.lc_provider()?.wallet_inst()?;
+				let parent = wallet.parent_key_id();
+				wallet.current_child_index(&parent)?
+			};
+			assert_eq!(current_child_index, child_index);
+
+			let (output, pos) = outputs
+				.iter()
+				.find_map(|output| {
+					let unused = !input_commits
+						.iter()
+						.any(|input| input.commitment() == output.commit)
+						&& !tx
+							.outputs()
+							.iter()
+							.any(|tx_output| tx_output.commitment() == output.commit);
+					if unused {
+						chain
+							.get_output_pos(&output.commit)
+							.ok()
+							.map(|pos| (output, pos))
+					} else {
+						None
+					}
+				})
+				.expect("sender has an unused confirmed output");
+			let features = if output.output.is_coinbase {
+				OutputFeatures::Coinbase
+			} else {
+				OutputFeatures::Plain
+			};
+			let mut changed = slate.clone();
+			changed.tx = Some(
+				changed
+					.tx_or_err()?
+					.clone()
+					.with_input(Input::new(features, output.commit)),
+			);
+			let view = api.contract_view(m, &changed)?;
+			assert_eq!(
+				view.own_commitment_status,
+				OwnCommitmentStatus::UnexpectedInput
+			);
+			let err = api
+				.contract_sign(m, &changed, &ContractSetupArgsAPI::default())
+				.unwrap_err();
+			match err {
+				libwallet::Error::GenericError(message) => assert_eq!(
+					message,
+					"Contract slate contains an unexpected input commitment from this wallet"
+				),
+				err => panic!("unexpected error: {}", err),
+			}
+
+			let mut changed = slate.clone();
+			changed.tx = Some(
+				changed
+					.tx_or_err()?
+					.clone()
+					.with_output(chain.get_unspent_output_at(pos).unwrap()),
+			);
+			let view = api.contract_view(m, &changed)?;
+			assert_eq!(
+				view.own_commitment_status,
+				OwnCommitmentStatus::UnexpectedOutput
+			);
+			let err = api
+				.contract_sign(m, &changed, &ContractSetupArgsAPI::default())
+				.unwrap_err();
+			match err {
+				libwallet::Error::GenericError(message) => assert_eq!(
+					message,
+					"Contract slate contains an unexpected output commitment from this wallet"
+				),
+				err => panic!("unexpected error: {}", err),
+			}
+
+			let mut changed = slate.clone();
+			changed.tx = Some(
+				changed
+					.tx_or_err()?
+					.clone()
+					.with_input(Input::new(features, output.commit))
+					.with_output(chain.get_unspent_output_at(pos).unwrap()),
+			);
+			let view = api.contract_view(m, &changed)?;
+			assert_eq!(
+				view.own_commitment_status,
+				OwnCommitmentStatus::UnexpectedInputAndOutput
+			);
+			let err = api
+				.contract_sign(m, &changed, &ContractSetupArgsAPI::default())
+				.unwrap_err();
+			match err {
+				libwallet::Error::GenericError(message) => assert_eq!(
+					message,
+					"Contract slate contains unexpected input and output commitments from this wallet"
+				),
+				err => panic!("unexpected error: {}", err),
+			}
+			Ok(())
+		},
+	)?;
 
 	// Send wallet finalizes and posts
 	wallet::controller::owner_single_use(
@@ -93,6 +259,26 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error>
 		},
 	)?;
 	assert_eq!(slate.state, SlateState::Standard3);
+	wallet::controller::owner_single_use(
+		recv_wallet.clone(),
+		recv_mask,
+		PathBuf::from(test_dir),
+		|api, m| {
+			let view = api.contract_view(m, &slate)?;
+			assert_eq!(view.own_commitment_status, OwnCommitmentStatus::Clean);
+			Ok(())
+		},
+	)?;
+	wallet::controller::owner_single_use(
+		send_wallet.clone(),
+		send_mask,
+		PathBuf::from(test_dir),
+		|api, m| {
+			let view = api.contract_view(m, &slate)?;
+			assert_eq!(view.own_commitment_status, OwnCommitmentStatus::Unknown);
+			Ok(())
+		},
+	)?;
 
 	wallet::controller::owner_single_use(
 		send_wallet.clone(),

doc/contracts.md

@@ -40,8 +40,11 @@ transaction has spent the available outputs first.
 ## View and revoke
 
 `view` reads a slate or Slatepack and shows the participants, signatures, suggested or
-agreed amount change, and whether the local transaction is confirmed. It does not show all
-inputs, outputs and fees, and it cannot find a contract by id.
+agreed amount change, whether the local transaction is confirmed, and whether it contains
+unexpected inputs or outputs from this wallet. This check is reported as unknown after
+the private context has been removed, when input features are missing, or when the slate
+does not contain a transaction. `view` does not show all inputs, outputs and fees, and it
+cannot find a contract by id.
 
 `revoke` cancels the local transaction. When the wallet added an input, it returns a
 self-spend of that input. The caller still has to post it, and either transaction can win
@@ -53,7 +56,6 @@ account selected with `--account`.
 ## Current limitations
 
 * Only one or two participants are supported
-* A returned slate is not yet checked against every input and output saved during setup
 * Early payment proofs are only implemented for contracts and are only available through
   the API. Only invoice proofs are supported and they require the experimental Slate V5
   format. The full proposal is described in

libwallet/src/contract/actions/README.md

@@ -31,7 +31,7 @@ only added to the slate when signing, so the counterparty does not see them earl
 Ideally we'd also separate side effects out of these functions e.g. computing the current_height
 or refreshing the outputs with updater::refresh_outputs(...). The current_height could be
 communicated through a &ChainState parameter which would collect these values before the call.
-Additionally, we could fetch the existing Context before the call to avoid doing db fetch.
+Setup still reads an existing Context again; passing it through would avoid that DB read.
 Separating side effects until the 'save_step' part would make these functions much easier to test.
 
 #### TODOs
@@ -41,8 +41,6 @@ Separating side effects until the 'save_step' part would make these functions mu
    only `--add-outputs` locks during `new`.
  - Reserve funds across open late-locked contracts, or make the possible signing failure
    clearer to callers.
- - Check the commitments we contributed are the ones that come back, so a counterparty
-   can't return a slate carrying inputs or outputs of ours that we didn't put there.
  - Handle the coinbase flows. Coinbase inputs are built into the slate, but nothing else
    is exercised.
  - For a payjoin, `--use-inputs any` picks an input for us; naming the commitment would be
@@ -77,7 +75,7 @@ Separating side effects until the 'save_step' part would make these functions mu
  - signing the same slate twice, which `verify_not_signed` guards
  - zero-value outputs, and `--make-outputs` beyond what is available
  - a receiver with no spendable input, through lack of funds or of confirmations
- - invalid or inconsistent slates, including changed commitments and unexpected states
+ - invalid or inconsistent slates and unexpected states
  - the CLI conflict between `--no-payjoin` and `--use-inputs`
  - a committed fee that no longer covers the selected inputs and outputs
  - slate contents at each step, not only the end state
@@ -102,6 +100,8 @@ Separating side effects until the 'save_step' part would make these functions mu
 
 #### Sign
 	// Side-effects:
-	//  - contract_utils::check_already_signed -> tx_log_iter
-	//  - contract_utils::get_net_change -> context and net_change
+	//  - contract_utils::verify_not_signed -> tx_log_iter
+	//  - sign -> w.get_private_context(...)
+	//  - verify_incoming_own_commitments -> w.iter()
+	//  - verify_own_commitments -> w.get(...)
 	//  - everything from 'setup'

libwallet/src/contract/actions/sign.rs

@@ -37,13 +37,23 @@ where
 {
 	// Compute if we will add outputs at this step. Only a missing context means we
 	// still need to add them; propagate any other error.
-	let will_add_outputs = match w.get_private_context(keychain_mask, slate.id.as_bytes()) {
-		Ok(ctx) => ctx.get_inputs().len() + ctx.get_outputs().len() == 0,
-		Err(Error::NotFoundErr(_)) => true,
+	let existing_context = match w.get_private_context(keychain_mask, slate.id.as_bytes()) {
+		Ok(context) => Some(context),
+		Err(Error::NotFoundErr(_)) => None,
 		Err(e) => return Err(e),
 	};
+	let will_add_outputs = existing_context
+		.as_ref()
+		.map(|context| context.get_inputs().len() + context.get_outputs().len() == 0)
+		.unwrap_or(true);
 	// Compute state for 'sign'
-	let (sl, mut context) = compute(w, keychain_mask, slate, setup_args)?;
+	let (sl, mut context) = compute(
+		w,
+		keychain_mask,
+		slate,
+		setup_args,
+		existing_context.as_ref(),
+	)?;
 
 	// Atomically commit state
 	contract::utils::save_step(w, keychain_mask, &sl, &mut context, will_add_outputs, true)?;
@@ -57,6 +67,7 @@ pub fn compute<C, K>(
 	keychain_mask: Option<&SecretKey>,
 	slate: &Slate,
 	setup_args: &ContractSetupArgsAPI,
+	existing_context: Option<&Context>,
 ) -> Result<(Slate, Context), Error>
 where
 	C: NodeClient,
@@ -67,18 +78,20 @@ where
 
 	// Ensure net_change has been provided
 	let expected_net_change =
-		contract::utils::get_net_change(w, keychain_mask, &sl.id, setup_args.net_change)?;
+		contract::utils::get_net_change(existing_context, setup_args.net_change)?;
 
 	// Define the values that must be provided in the setup phase at the sign step
 	let mut setup_args = setup_args.clone();
 	setup_args.net_change = Some(expected_net_change);
 	setup_args.num_participants = sl.num_participants;
 	setup_args.add_outputs = true; // we add outputs to the Context in case we haven't done that yet
+	contract::slate::verify_incoming_own_commitments(w, keychain_mask, &sl)?;
 
 	// Ensure Setup phase is done and that inputs/outputs have been added to the Context
 	let (mut sl, mut context) = setup::compute(w, keychain_mask, &mut sl, &setup_args)?;
 	// Add outputs to the slate, verify the payment proof and sign the slate
 	contract::slate::add_outputs(w, keychain_mask, &mut sl, &context)?;
+	contract::slate::verify_own_commitments(w, keychain_mask, &sl, &context)?;
 	contract::slate::verify_invoice_promise(&sl, &w.keychain(keychain_mask)?, &context)?;
 
 	contract::slate::sign(w, keychain_mask, &mut sl, &mut context)?;

libwallet/src/contract/actions/view.rs

@@ -15,7 +15,8 @@
 //! Implementation of contract view
 
 use crate::backend::WalletBackend;
-use crate::contract::types::ContractView;
+use crate::contract;
+use crate::contract::types::{ContractView, OwnCommitmentStatus};
 use crate::error::Error;
 use crate::grin_keychain::Keychain;
 use crate::grin_util::secp::key::SecretKey;
@@ -63,13 +64,32 @@ where
 			slate.num_participants
 		)));
 	}
+	let context = match w.get_private_context(keychain_mask, slate.id.as_bytes()) {
+		Ok(context) => Some(context),
+		Err(Error::NotFoundErr(_)) => None,
+		Err(e) => return Err(e),
+	};
 	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.
-	let is_executed = updater::retrieve_txs(w, None, Some(slate.id), None, None, false)?
+	let txs = updater::retrieve_txs(w, None, Some(slate.id), None, None, false)?;
+	let is_executed = txs.iter().any(|tx| tx.confirmed);
+	let has_signed = txs
 		.iter()
-		.any(|tx| tx.confirmed);
+		.any(|tx| contract::utils::is_signed_tx(tx, slate.id));
+	// Once we have signed, the context identifies the commitments we added. If it has
+	// already been removed, the slate no longer carries enough information to do that.
+	let own_commitment_status = if slate.tx.is_none() || (context.is_none() && !txs.is_empty()) {
+		OwnCommitmentStatus::Unknown
+	} else {
+		contract::slate::own_commitment_status(
+			w,
+			keychain_mask,
+			slate,
+			if has_signed { context.as_ref() } else { None },
+		)?
+	};
 	// Count signatures present (a participant is "complete" once it has a partial sig).
 	let num_sigs = slate
 		.participant_data
@@ -79,11 +99,10 @@ where
 		.count();
 
 	// If we have a local context for this slate we've agreed on a net change; surface it.
-	let agreed_net_change = match w.get_private_context(keychain_mask, slate.id.as_bytes()) {
-		Ok(ctx) => ctx.setup_args.as_ref().and_then(|args| args.net_change),
-		Err(Error::NotFoundErr(_)) => None,
-		Err(e) => return Err(e),
-	};
+	let agreed_net_change = context
+		.as_ref()
+		.and_then(|context| context.setup_args.as_ref())
+		.and_then(|args| args.net_change);
 
 	// TODO: Maybe we can know if the slate was meant for us if it was encrypted for us.
 	// A possible issue is that one can encrypt the same slate for 10 people.
@@ -93,6 +112,7 @@ where
 		agreed_net_change,
 		num_sigs: num_sigs as u8,
 		is_executed: is_executed,
+		own_commitment_status,
 		..Default::default()
 	};
 	Ok(ct_view)

libwallet/src/contract/slate.rs

@@ -15,16 +15,21 @@
 //! Contract functions on the Slate
 
 use crate::backend::WalletBackend;
+use crate::grin_core::core::transaction::{Inputs, OutputFeatures, OutputIdentifier};
 use crate::grin_core::libtx::build;
 use crate::grin_core::libtx::proof::ProofBuilder;
-use crate::grin_keychain::Keychain;
+use crate::grin_keychain::{Identifier, Keychain, SwitchCommitmentType};
+use crate::grin_util::from_hex;
+use crate::grin_util::secp::constants::PEDERSEN_COMMITMENT_SIZE;
 use crate::grin_util::secp::key::{PublicKey, SecretKey};
+use crate::grin_util::secp::pedersen::Commitment;
 use crate::slate::{PaymentProofType, Slate, SlateState};
-use crate::types::{Context, NodeClient};
+use crate::types::{Context, NodeClient, OutputData};
 use crate::util::OnionV3Address;
 use crate::Error;
+use std::collections::BTreeSet;
 
-use super::types::ProofArgs;
+use super::types::{OwnCommitmentStatus, ProofArgs};
 use crate::contract::proofs::InvoiceProof;
 
 /// Add payment proof data to slate, noop for sender
@@ -121,6 +126,243 @@ where
 	Ok(())
 }
 
+fn derive_commitment<K>(keychain: &K, id: &Identifier, amount: u64) -> Result<Commitment, Error>
+where
+	K: Keychain,
+{
+	keychain
+		.commit(amount, id, SwitchCommitmentType::Regular)
+		.map_err(Error::from)
+}
+
+fn cached_commitment(commit: &str) -> Option<Commitment> {
+	from_hex(commit)
+		.ok()
+		.filter(|bytes| bytes.len() == PEDERSEN_COMMITMENT_SIZE)
+		.map(Commitment::from_vec)
+}
+
+fn stored_commitment<K>(keychain: &K, output: &OutputData) -> Result<Commitment, Error>
+where
+	K: Keychain,
+{
+	// Commitments written by the wallet are used directly. Rebuild malformed cache entries.
+	if let Some(commit) = output.commit.as_deref().and_then(cached_commitment) {
+		return Ok(commit);
+	}
+	derive_commitment(keychain, &output.key_id, output.value)
+}
+
+fn derive_identifier<K>(
+	keychain: &K,
+	id: &Identifier,
+	amount: u64,
+	features: OutputFeatures,
+) -> Result<OutputIdentifier, Error>
+where
+	K: Keychain,
+{
+	Ok(OutputIdentifier::new(
+		features,
+		&derive_commitment(keychain, id, amount)?,
+	))
+}
+
+fn check_identifiers(
+	actual: &[OutputIdentifier],
+	expected: &[OutputIdentifier],
+	kind: &str,
+) -> Result<(), Error> {
+	for expected in expected {
+		let mut matches = actual
+			.iter()
+			.filter(|actual| actual.commitment() == expected.commitment());
+		match matches.next() {
+			None => {
+				return Err(Error::GenericError(format!(
+					"Contract slate is missing one of our {} commitments",
+					kind
+				)))
+			}
+			Some(actual) if actual != expected || matches.next().is_some() => {
+				return Err(Error::GenericError(format!(
+					"Contract slate changed or duplicated one of our {} commitments",
+					kind
+				)))
+			}
+			Some(_) => {}
+		}
+	}
+	Ok(())
+}
+
+fn expected_identifiers<C, K>(
+	w: &WalletBackend<C, K>,
+	keychain: &K,
+	context: &Context,
+) -> Result<(Vec<OutputIdentifier>, Vec<OutputIdentifier>), Error>
+where
+	C: NodeClient,
+	K: Keychain,
+{
+	let expected_inputs = context
+		.input_ids
+		.iter()
+		.map(|(id, mmr_index, amount)| {
+			let output = w.get(id, mmr_index)?;
+			derive_identifier(
+				keychain,
+				id,
+				*amount,
+				if output.is_coinbase {
+					OutputFeatures::Coinbase
+				} else {
+					OutputFeatures::Plain
+				},
+			)
+		})
+		.collect::<Result<Vec<_>, Error>>()?;
+	let expected_outputs = context
+		.output_ids
+		.iter()
+		.map(|(id, _, amount)| derive_identifier(keychain, id, *amount, OutputFeatures::Plain))
+		.collect::<Result<Vec<_>, Error>>()?;
+	Ok((expected_inputs, expected_outputs))
+}
+
+const MISSING_INPUT_FEATURES: &str = "Contract slate input features are missing";
+
+/// Compare a slate with commitments owned by this wallet. A signed context
+/// identifies commitments that this wallet has already added.
+pub(super) fn own_commitment_status<C, K>(
+	w: &WalletBackend<C, K>,
+	keychain_mask: Option<&SecretKey>,
+	slate: &Slate,
+	signed_context: Option<&Context>,
+) -> Result<OwnCommitmentStatus, Error>
+where
+	C: NodeClient,
+	K: Keychain,
+{
+	let keychain = w.keychain(keychain_mask)?;
+	let tx = slate.tx_or_err()?;
+	let inputs = match tx.inputs() {
+		// Unknown only means missing input features here. View also uses Unknown when
+		// the private context has already been removed.
+		Inputs::CommitOnly(_) => return Ok(OwnCommitmentStatus::Unknown),
+		Inputs::FeaturesAndCommit(inputs) => inputs,
+	};
+	let (expected_inputs, expected_outputs) = match signed_context {
+		Some(context) => {
+			let (inputs, outputs) = expected_identifiers(w, &keychain, context)?;
+			(
+				inputs
+					.into_iter()
+					.map(|input| input.commitment())
+					.collect::<BTreeSet<_>>(),
+				outputs
+					.into_iter()
+					.map(|output| output.commitment())
+					.collect::<BTreeSet<_>>(),
+			)
+		}
+		None => (BTreeSet::new(), BTreeSet::new()),
+	};
+	let incoming_inputs = inputs
+		.into_iter()
+		.map(|input| input.commitment())
+		.filter(|commit| !expected_inputs.contains(commit))
+		.collect::<BTreeSet<_>>();
+	let incoming_outputs = tx
+		.outputs()
+		.iter()
+		.map(|output| output.commitment())
+		.filter(|commit| !expected_outputs.contains(commit))
+		.collect::<BTreeSet<_>>();
+	if incoming_inputs.is_empty() && incoming_outputs.is_empty() {
+		return Ok(OwnCommitmentStatus::Clean);
+	}
+
+	let mut unexpected_input = false;
+	let mut unexpected_output = false;
+	for output in w.iter()? {
+		let commit = stored_commitment(&keychain, &output)?;
+		unexpected_input |= incoming_inputs.contains(&commit);
+		unexpected_output |= incoming_outputs.contains(&commit);
+		if (unexpected_input || incoming_inputs.is_empty())
+			&& (unexpected_output || incoming_outputs.is_empty())
+		{
+			break;
+		}
+	}
+	Ok(match (unexpected_input, unexpected_output) {
+		(true, true) => OwnCommitmentStatus::UnexpectedInputAndOutput,
+		(true, false) => OwnCommitmentStatus::UnexpectedInput,
+		(false, true) => OwnCommitmentStatus::UnexpectedOutput,
+		(false, false) => OwnCommitmentStatus::Clean,
+	})
+}
+
+/// Reject commitments from this wallet before contract setup reserves any keys.
+pub(super) fn verify_incoming_own_commitments<C, K>(
+	w: &WalletBackend<C, K>,
+	keychain_mask: Option<&SecretKey>,
+	slate: &Slate,
+) -> Result<(), Error>
+where
+	C: NodeClient,
+	K: Keychain,
+{
+	let found = match own_commitment_status(w, keychain_mask, slate, None)? {
+		OwnCommitmentStatus::UnexpectedInput => "an unexpected input commitment",
+		OwnCommitmentStatus::UnexpectedOutput => "an unexpected output commitment",
+		OwnCommitmentStatus::UnexpectedInputAndOutput => "unexpected input and output commitments",
+		OwnCommitmentStatus::Unknown => {
+			return Err(Error::GenericError(MISSING_INPUT_FEATURES.to_string()))
+		}
+		OwnCommitmentStatus::Clean => return Ok(()),
+	};
+	Err(Error::GenericError(format!(
+		"Contract slate contains {} from this wallet",
+		found
+	)))
+}
+
+/// Check that the slate still contains the commitments selected for this contract.
+pub(super) fn verify_own_commitments<C, K>(
+	w: &WalletBackend<C, K>,
+	keychain_mask: Option<&SecretKey>,
+	slate: &Slate,
+	context: &Context,
+) -> Result<(), Error>
+where
+	C: NodeClient,
+	K: Keychain,
+{
+	let keychain = w.keychain(keychain_mask)?;
+	let (expected_inputs, expected_outputs) = expected_identifiers(w, &keychain, context)?;
+	let tx = slate.tx_or_err()?;
+	match tx.inputs() {
+		Inputs::CommitOnly(_) => {
+			return Err(Error::GenericError(MISSING_INPUT_FEATURES.to_string()))
+		}
+		Inputs::FeaturesAndCommit(inputs) => {
+			let inputs = inputs
+				.iter()
+				.map(OutputIdentifier::from)
+				.collect::<Vec<_>>();
+			check_identifiers(&inputs, &expected_inputs, "input")?;
+		}
+	}
+	let actual_outputs = tx
+		.outputs()
+		.iter()
+		.map(|output| output.identifier())
+		.collect::<Vec<_>>();
+
+	check_identifiers(&actual_outputs, &expected_outputs, "output")
+}
+
 /// Contribute inputs to slate
 fn add_inputs_to_slate<C, K>(
 	w: &mut WalletBackend<C, K>,
@@ -388,4 +630,26 @@ mod tests {
 			assert_eq!(slate.state, to);
 		}
 	}
+
+	#[test]
+	fn checks_own_commitments() {
+		let identifier =
+			|value, features| OutputIdentifier::new(features, &Commitment::from_vec(vec![value]));
+		let own_input = identifier(1, OutputFeatures::Plain);
+		let own_output = identifier(2, OutputFeatures::Plain);
+
+		check_identifiers(&[own_input], &[own_input], "input").unwrap();
+		assert!(check_identifiers(&[], &[own_input], "input").is_err());
+		assert!(check_identifiers(&[own_input], &[own_output], "output").is_err());
+		assert!(check_identifiers(&[own_input, own_input], &[own_input], "input").is_err());
+		assert!(check_identifiers(
+			&[identifier(1, OutputFeatures::Coinbase)],
+			&[own_input],
+			"input"
+		)
+		.is_err());
+		assert!(cached_commitment("01").is_none());
+		assert!(cached_commitment("not hex").is_none());
+		assert!(cached_commitment(&"00".repeat(PEDERSEN_COMMITMENT_SIZE)).is_some());
+	}
 }

libwallet/src/contract/types.rs

@@ -179,6 +179,29 @@ pub struct ContractView {
 	pub num_sigs: u8,
 	/// Has the contract been executed on chain
 	pub is_executed: bool,
+	/// Whether the slate contains an unexpected commitment from this wallet.
+	/// This is unknown when the private context is no longer available, input
+	/// features are missing, or the slate contains no transaction.
+	#[serde(default)]
+	pub own_commitment_status: OwnCommitmentStatus,
+}
+
+/// Result of comparing a contract slate with commitments owned by this wallet
+#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Serialize, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum OwnCommitmentStatus {
+	/// No unexpected wallet commitment was found
+	Clean,
+	/// The slate contains an unexpected input from this wallet
+	UnexpectedInput,
+	/// The slate contains an unexpected output from this wallet
+	UnexpectedOutput,
+	/// The slate contains an unexpected input and output from this wallet
+	UnexpectedInputAndOutput,
+	/// The comparison could not be made
+	#[default]
+	#[serde(other)]
+	Unknown,
 }
 
 impl Default for ContractView {
@@ -189,6 +212,7 @@ impl Default for ContractView {
 			agreed_net_change: None,
 			num_sigs: 0,
 			is_executed: false,
+			own_commitment_status: OwnCommitmentStatus::Unknown,
 		}
 	}
 }
@@ -201,3 +225,43 @@ pub struct ContractRevokeArgsAPI {
 	/// Account containing the transaction, or the active account when omitted
 	pub src_acct_name: Option<String>,
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+
+	#[test]
+	fn own_commitment_status_json() {
+		for (status, value) in [
+			(OwnCommitmentStatus::Clean, "clean"),
+			(OwnCommitmentStatus::UnexpectedInput, "unexpected_input"),
+			(OwnCommitmentStatus::UnexpectedOutput, "unexpected_output"),
+			(
+				OwnCommitmentStatus::UnexpectedInputAndOutput,
+				"unexpected_input_and_output",
+			),
+			(OwnCommitmentStatus::Unknown, "unknown"),
+		] {
+			let json = format!("\"{}\"", value);
+			assert_eq!(serde_json::to_string(&status).unwrap(), json);
+			assert_eq!(
+				serde_json::from_str::<OwnCommitmentStatus>(&json).unwrap(),
+				status
+			);
+		}
+		assert_eq!(
+			serde_json::from_str::<OwnCommitmentStatus>("\"future_status\"").unwrap(),
+			OwnCommitmentStatus::Unknown
+		);
+
+		let view: ContractView = serde_json::from_value(serde_json::json!({
+			"num_participants": 2,
+			"suggested_net_change": null,
+			"agreed_net_change": null,
+			"num_sigs": 0,
+			"is_executed": false
+		}))
+		.unwrap();
+		assert_eq!(view.own_commitment_status, OwnCommitmentStatus::Unknown);
+	}
+}

libwallet/src/contract/utils.rs

@@ -136,41 +136,31 @@ where
 }
 
 /// Get net_change value. This is obtained either from the Context.net_change or the setup_args.net_change
-pub fn get_net_change<C, K>(
-	w: &mut WalletBackend<C, K>,
-	keychain_mask: Option<&SecretKey>,
-	slate_id: &Uuid,
+pub fn get_net_change(
+	context: Option<&Context>,
 	setup_args_net_change: Option<i64>,
-) -> Result<i64, Error>
-where
-	C: NodeClient,
-	K: Keychain,
-{
+) -> Result<i64, Error> {
 	let mut expected_net_change: Option<i64> = setup_args_net_change;
-	match w.get_private_context(keychain_mask, slate_id.as_bytes()) {
-		Ok(ctx) => {
-			debug!("contract::sign => context found");
-			// We have a context so we must have agreed on a certain net_change value in Context.net_change.
-			// If we have both Context.net_change and setup_args.net_change, then they must be equal.
-			let ctx_net_change = ctx.get_net_change()?;
-			match expected_net_change {
-				Some(args_net_change) => {
-					if ctx_net_change != args_net_change {
-						return Err(Error::GenericError(format!(
-							"Expected net change mismatch! Context.net_change: {}, setup_args.net_change: {}",
-							ctx_net_change, args_net_change
-						)));
-					}
+	if let Some(context) = context {
+		debug!("contract::sign => context found");
+		// We have a context so we must have agreed on a certain net_change value in Context.net_change.
+		// If we have both Context.net_change and setup_args.net_change, then they must be equal.
+		let ctx_net_change = context.get_net_change()?;
+		match expected_net_change {
+			Some(args_net_change) => {
+				if ctx_net_change != args_net_change {
+					return Err(Error::GenericError(format!(
+						"Expected net change mismatch! Context.net_change: {}, setup_args.net_change: {}",
+						ctx_net_change, args_net_change
+					)));
 				}
-				None => (),
 			}
-			expected_net_change = Some(ctx_net_change);
+			None => (),
 		}
-		Err(Error::NotFoundErr(_)) => {
-			debug!("contract::utils::get_net_change => context not found")
-		}
-		Err(e) => return Err(e),
-	};
+		expected_net_change = Some(ctx_net_change);
+	} else {
+		debug!("contract::utils::get_net_change => context not found")
+	}
 
 	// Fail if net_change was not passed to setup_args and was also not present in the context.
 	// This means it has not been explicitly agreed on and we require the user to pass it.
@@ -357,6 +347,11 @@ pub fn my_fee_contribution(
 	Ok(my_fee_fields)
 }
 
+/// Whether a transaction log entry shows that this wallet signed the slate.
+pub(super) fn is_signed_tx(tx: &TxLogEntry, slate_id: Uuid) -> bool {
+	tx.tx_slate_id == Some(slate_id) && tx.kernel_excess.is_some()
+}
+
 /// Returns an error if the slate has already been signed (in our local database). Even if the
 /// result is Ok, it's still possible it was signed but we don't have the data about it locally.
 pub fn verify_not_signed<C, K>(w: &mut WalletBackend<C, K>, slate_id: Uuid) -> Result<(), Error>
@@ -368,7 +363,7 @@ where
 	// we have already signed this slate.
 	for tx in w.tx_log_iter()? {
 		let tx = tx?;
-		if tx.tx_slate_id == Some(slate_id) && tx.kernel_excess.is_some() {
+		if is_signed_tx(&tx, slate_id) {
 			debug!("contract::utils::verify_not_signed => The slate has already been signed.");
 			return Err(Error::GenericError(format!(
 				"Slate with id:{} has already been signed.",