fix early proof export and slatepack account errors

controller/src/command.rs

@@ -27,8 +27,8 @@ use crate::libwallet::contract::types::{
 };
 use crate::libwallet::contract::{can_finalize, initial_net_change};
 use crate::libwallet::{
-	self, InitTxArgs, IssueInvoiceTxArgs, NodeClient, PaymentProof, Slate, SlateState,
+	self, EarlyPaymentProof, InitTxArgs, IssueInvoiceTxArgs, NodeClient, PaymentProof, Slate,
-	SlatepackAddress, Slatepacker, SlatepackerArgs, WalletLCProvider,
+	SlateState, SlatepackAddress, Slatepacker, SlatepackerArgs, WalletLCProvider,
 };
 use crate::util::secp::key::SecretKey;
 use crate::util::{Mutex, ZeroingString};
@@ -774,7 +774,22 @@ where
 	let was_encrypted = slatepack.is_encrypted();
 	if was_encrypted {
 		let dec_key = owner_api.get_slatepack_secret_key(keychain_mask, 0)?;
-		slatepack.try_decrypt_payload(Some(&dec_key))?;
+		if let Err(e) = slatepack.try_decrypt_payload(Some(&dec_key)) {
+			// tell the user which account can decrypt it
+			let account = libwallet::api_impl::owner::slatepack_account(
+				owner_api.wallet_inst.clone(),
+				keychain_mask,
+				&slatepack,
+			)?;
+			return Err(match account {
+				Some(label) => libwallet::Error::SlatepackDecryption(format!(
+					"it is encrypted for account '{}' of this wallet, select it with --account {}",
+					label, label
+				))
+				.into(),
+				None => e.into(),
+			});
+		}
 	}
 	let slate = packer.get_slate(&slatepack)?;
 	Ok((slate, slatepack.sender, was_encrypted))
@@ -1554,6 +1569,13 @@ pub struct ProofExportArgs {
 	pub tx_slate_id: Option<Uuid>,
 }
 
+/// Exported early proof, with the receiver address needed to verify it
+#[derive(Serialize, Deserialize)]
+struct EarlyProofFile {
+	recipient_address: SlatepackAddress,
+	proof: EarlyPaymentProof,
+}
+
 pub fn proof_export<L, C, K>(
 	owner_api: &mut Owner<L, C, K>,
 	keychain_mask: Option<&SecretKey>,
@@ -1564,14 +1586,34 @@ where
 	C: NodeClient + 'static,
 	K: keychain::Keychain + 'static,
 {
-	let result = owner_api.retrieve_payment_proof(keychain_mask, true, args.id, args.tx_slate_id);
+	// early proofs have no sender signature
+	let (_, txs) = owner_api.retrieve_txs(keychain_mask, false, args.id, args.tx_slate_id, None)?;
+	let early_recipient = txs
+		.first()
+		.and_then(|tx| tx.payment_proof.as_ref())
+		.filter(|p| p.proof_type.is_some())
+		.map(|p| SlatepackAddress::new(&p.receiver_address));
+	let result = match early_recipient {
+		Some(recipient_address) => owner_api
+			.retrieve_payment_proof_early(keychain_mask, true, args.id, args.tx_slate_id)
+			.map(|proof| {
+				json::to_string_pretty(&EarlyProofFile {
+					recipient_address,
+					proof,
+				})
+				.unwrap()
+			}),
+		None => owner_api
+			.retrieve_payment_proof(keychain_mask, true, args.id, args.tx_slate_id)
+			.map(|p| json::to_string_pretty(&p).unwrap()),
+	};
 	match result {
 		Ok(p) => {
 			// actually export proof
 			let mut proof_file = File::create(args.output_file.clone())
 				.map_err(|e| Error::GenericError(format!("{}", e)))?;
 			proof_file
-				.write_all(json::to_string_pretty(&p).unwrap().as_bytes())
+				.write_all(p.as_bytes())
 				.map_err(|e| Error::GenericError(format!("{}", e)))?;
 			proof_file
 				.sync_all()
@@ -1616,6 +1658,13 @@ where
 	proof_f
 		.read_to_string(&mut proof)
 		.map_err(|e| Error::GenericError(format!("{}", e)))?;
+	// early proof file
+	let is_early = json::from_str::<json::Value>(&proof)
+		.map(|v| v.get("proof").is_some())
+		.unwrap_or(false);
+	if is_early {
+		return proof_verify_early(owner_api, keychain_mask, &proof);
+	}
 	// read
 	let proof: PaymentProof = match json::from_str(&proof) {
 		Ok(p) => p,
@@ -1628,18 +1677,7 @@ where
 	let result = owner_api.verify_payment_proof(keychain_mask, &proof);
 	match result {
 		Ok((iam_sender, iam_recipient)) => {
-			println!("Payment proof's signatures are valid.");
+			print_proof_verified(iam_sender, iam_recipient);
-			if iam_sender {
-				println!("The proof's sender address belongs to this wallet.");
-			}
-			if iam_recipient {
-				println!("The proof's recipient address belongs to this wallet.");
-			}
-			if !iam_recipient && !iam_sender {
-				println!(
-					"Neither the proof's sender nor recipient address belongs to this wallet."
-				);
-			}
 			Ok(())
 		}
 		Err(e) => {
@@ -1649,6 +1687,54 @@ where
 	}
 }
 
+fn print_proof_verified(iam_sender: bool, iam_recipient: bool) {
+	println!("Payment proof's signatures are valid.");
+	if iam_sender {
+		println!("The proof's sender address belongs to this wallet.");
+	}
+	if iam_recipient {
+		println!("The proof's recipient address belongs to this wallet.");
+	}
+	if !iam_recipient && !iam_sender {
+		println!("Neither the proof's sender nor recipient address belongs to this wallet.");
+	}
+}
+
+fn proof_verify_early<L, C, K>(
+	owner_api: &mut Owner<L, C, K>,
+	keychain_mask: Option<&SecretKey>,
+	proof: &str,
+) -> Result<(), Error>
+where
+	L: WalletLCProvider<'static, C, K> + 'static,
+	C: NodeClient + 'static,
+	K: keychain::Keychain + 'static,
+{
+	let file: EarlyProofFile = json::from_str(proof).map_err(|e| {
+		error!("Unable to parse payment proof file: {}", e);
+		Error::from(libwallet::Error::PaymentProofParsing(format!("{}", e)))
+	})?;
+	let result = {
+		let mut w_lock = owner_api.wallet_inst.lock();
+		let w = w_lock.lc_provider()?.wallet_inst()?;
+		libwallet::api_impl::foreign::verify_payment_proof_early(
+			w,
+			&file.recipient_address.pub_key,
+			&file.proof,
+		)
+	};
+	if let Err(e) = result {
+		error!("Proof not valid: {}", e);
+		return Err(Error::from(e));
+	}
+	let my_address = owner_api.get_slatepack_address(keychain_mask, 0)?.pub_key;
+	print_proof_verified(
+		file.proof.sender_address == my_address,
+		file.recipient_address.pub_key == my_address,
+	);
+	Ok(())
+}
+
 /// Create new contract command arguments
 #[derive(Clone)]
 pub struct ContractNewArgs {

controller/src/display.rs

@@ -598,14 +598,7 @@ pub fn payment_proof(tx: &TxLogEntry) -> Result<(), Error> {
 		Some(f) => f.fee(), // apply fee mask past HF4
 		None => 0,
 	};
-	let amount = if tx.amount_credited >= tx.amount_debited {
+	let amount = core::amount_to_hr_string(proof_amount(tx, pp.proof_type.is_some(), fee), true);
-		core::amount_to_hr_string(tx.amount_credited - tx.amount_debited, true)
-	} else {
-		format!(
-			"{}",
-			core::amount_to_hr_string(tx.amount_debited - tx.amount_credited - fee, true)
-		)
-	};
 
 	let sender_signature = match pp.sender_signature {
 		Some(s) => {
@@ -647,6 +640,15 @@ pub fn payment_proof(tx: &TxLogEntry) -> Result<(), Error> {
 	Ok(())
 }
 
+/// Contract tx logs store the net amount, so early proofs don't subtract the fee
+fn proof_amount(tx: &TxLogEntry, early_proof: bool, fee: u64) -> u64 {
+	if early_proof || tx.amount_credited >= tx.amount_debited {
+		tx.amount_credited.abs_diff(tx.amount_debited)
+	} else {
+		tx.amount_debited - tx.amount_credited - fee
+	}
+}
+
 /// Display a summary of a contract slate
 pub fn contract_view(slate: &Slate, view: &ContractView, was_encrypted: bool) {
 	println!("\n____ Contract ____\n");
@@ -766,11 +768,12 @@ fn format_net_change(change: i64) -> String {
 
 #[cfg(test)]
 mod tests {
-	use super::{contract_view_table, format_net_change};
+	use super::{contract_view_table, format_net_change, proof_amount};
 	use crate::core::core::{FeeFields, Input, Inputs, Output, OutputFeatures, Transaction};
 	use crate::core::global;
+	use crate::keychain::Identifier;
 	use crate::libwallet::contract::types::ContractView;
-	use crate::libwallet::Slate;
+	use crate::libwallet::{Slate, TxLogEntry, TxLogEntryType};
 	use crate::util::secp::pedersen::{Commitment, RangeProof};
 
 	fn table_value(table: &prettytable::Table, label: &str) -> String {
@@ -794,6 +797,19 @@ mod tests {
 		assert_eq!(format_net_change(0), "+0.000000000");
 	}
 
+	#[test]
+	fn proof_amount_legacy_and_early() {
+		let mut tx = TxLogEntry::new(Identifier::zero(), TxLogEntryType::TxSent, 0);
+		// legacy send
+		tx.amount_debited = 1_500_000_000;
+		tx.amount_credited = 477_000_000;
+		assert_eq!(proof_amount(&tx, false, 23_000_000), 1_000_000_000);
+		// contract send
+		tx.amount_debited = 200_000_000;
+		tx.amount_credited = 0;
+		assert_eq!(proof_amount(&tx, true, 11_750_000), 200_000_000);
+	}
+
 	#[test]
 	fn contract_view_rows() {
 		global::set_local_chain_type(global::ChainTypes::AutomatedTesting);

controller/tests/contract_early_proofs_srs.rs

@@ -240,6 +240,49 @@ fn contract_early_proofs_srs_test_impl(test_dir: &'static str) -> Result<(), lib
 		},
 	)?;
 
+	// export and verify through the CLI
+	let proof_file = format!("{}/early_proof.json", test_dir);
+	let tampered_file = format!("{}/early_proof_tampered.json", test_dir);
+	wallet::controller::owner_single_use(
+		send_wallet.clone(),
+		send_mask,
+		PathBuf::from(test_dir),
+		|api, m| {
+			wallet::command::proof_export(
+				api,
+				m,
+				wallet::command::ProofExportArgs {
+					output_file: proof_file.clone(),
+					id: None,
+					tx_slate_id: Some(slate.id),
+				},
+			)
+			.map_err(|e| libwallet::Error::GenericError(e.to_string()))?;
+			wallet::command::proof_verify(
+				api,
+				m,
+				wallet::command::ProofVerifyArgs {
+					input_file: proof_file.clone(),
+				},
+			)
+			.map_err(|e| libwallet::Error::GenericError(e.to_string()))?;
+			// tweak the amount and it shouldn't verify
+			let mut tampered: serde_json::Value =
+				serde_json::from_str(&std::fs::read_to_string(&proof_file).unwrap()).unwrap();
+			tampered["proof"]["amount"] = serde_json::Value::from("400000");
+			std::fs::write(&tampered_file, tampered.to_string()).unwrap();
+			let retval = wallet::command::proof_verify(
+				api,
+				m,
+				wallet::command::ProofVerifyArgs {
+					input_file: tampered_file.clone(),
+				},
+			);
+			assert!(retval.is_err());
+			Ok(())
+		},
+	)?;
+
 	// let logging finish
 	stopper.store(false, Ordering::Relaxed);
 	thread::sleep(Duration::from_millis(200));

controller/tests/contract_rsr.rs

@@ -224,6 +224,7 @@ fn contract_rsr_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error>
 			assert_eq!(txs.len(), 5); // 4 mined and 1 received
 			let tx_log = common::tx_log_for_slate(api, m, &slate)?;
 			assert_eq!(tx_log.tx_type, TxLogEntryType::TxReceived);
+			assert_eq!(tx_log.tx_slate_state, Some(SlateState::Invoice3));
 			assert_eq!(tx_log.amount_credited, 5_000_000_000);
 			assert_eq!(tx_log.amount_debited, 0);
 			assert_eq!(tx_log.num_inputs, 1);
@@ -251,6 +252,7 @@ fn contract_rsr_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error>
 			assert_eq!(txs.len() as u64, bh - 4 + 1); // send_wallet didn't mine 4 blocks and made 1 tx
 			let tx_log = common::tx_log_for_slate(api, m, &slate)?;
 			assert_eq!(tx_log.tx_type, TxLogEntryType::TxSent);
+			assert_eq!(tx_log.tx_slate_state, Some(SlateState::Invoice2));
 			assert_eq!(tx_log.amount_credited, 0);
 			assert_eq!(tx_log.amount_debited, 5_000_000_000);
 			assert_eq!(tx_log.num_inputs, 1);

controller/tests/contract_srs.rs

@@ -631,6 +631,7 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error>
 			assert_eq!(txs.len(), 5); // 4 mined and 1 received
 			let tx_log = common::tx_log_for_slate(api, m, &slate)?;
 			assert_eq!(tx_log.tx_type, TxLogEntryType::TxReceived);
+			assert_eq!(tx_log.tx_slate_state, Some(SlateState::Standard2));
 			assert_eq!(tx_log.amount_credited, 5_000_000_000);
 			assert_eq!(tx_log.amount_debited, 0);
 			assert_eq!(tx_log.num_inputs, 1);
@@ -657,6 +658,7 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error>
 			assert_eq!(txs.len() as u64, bh - 4 + 1); // send wallet didn't mine 4 blocks and made 1 tx
 			let tx_log = common::tx_log_for_slate(api, m, &slate)?;
 			assert_eq!(tx_log.tx_type, TxLogEntryType::TxSent);
+			assert_eq!(tx_log.tx_slate_state, Some(SlateState::Standard3));
 			assert_eq!(tx_log.amount_credited, 0);
 			assert_eq!(tx_log.amount_debited, 5_000_000_000);
 			assert_eq!(tx_log.num_inputs, 1);

controller/tests/slatepack.rs

@@ -609,6 +609,17 @@ fn slatepack_api_impl(test_dir: &'static str) -> Result<(), libwallet::Error> {
 			println!("{}", slatepack_raw);
 			let decoded_slate = api.slate_from_slatepack_message(m, slatepack, vec![0])?;
 			println!("{}", decoded_slate);
+			// wrong active account, the error names the right one
+			let own_address = api.get_slatepack_address(m, 0)?;
+			let own_slatepack =
+				api.create_slatepack_message(m, &slate, Some(0), vec![own_address])?;
+			api.create_account_path(m, "second")?;
+			api.set_active_account(m, "second")?;
+			let err = api
+				.slate_from_slatepack_message(m, own_slatepack, vec![0])
+				.unwrap_err();
+			assert!(err.to_string().contains("account 'default'"), "{}", err);
+			api.set_active_account(m, "default")?;
 			Ok(())
 		},
 	)?;

doc/contracts.md

@@ -52,6 +52,7 @@ does not, revoke the contract and create a new one.
 
 Each side's account is fixed when it first joins the contract. Changing the active account
 later does not affect setup or signing; they use the account stored in the context.
+Encrypted Slatepacks still need their account selected with `--account`; the error names it.
 
 ## View and revoke
 
@@ -80,6 +81,7 @@ account selected with `--account`.
   the [known multi-party attack](https://forum.grin.mw/t/grin-wallet-contract-prototype/9745/18)
 * Early payment proofs are available for contracts through the API and CLI. Use `--proof-type`
   and optionally `--memo` (up to 1024 bytes of UTF-8 text). The proof binds the memo's Blake2b hash
+* `export_proof` and `verify_proof` also handle early proofs
 * Invoice proofs work with SRS and RSR. Sender-nonce proofs only work with RSR. Both need
   experimental Slate V5; see
   [Early Payment Proofs](https://github.com/mimblewimble/grin-rfcs/pull/70)

libwallet/src/api_impl/owner.rs

@@ -142,6 +142,31 @@ where
 	Ok(d_skey)
 }
 
+/// Label of another account that can decrypt the slatepack, if any
+pub fn slatepack_account<'a, L, C, K>(
+	wallet_inst: Arc<Mutex<Box<dyn WalletInst<'a, L, C, K>>>>,
+	keychain_mask: Option<&SecretKey>,
+	slatepack: &Slatepack,
+) -> Result<Option<String>, Error>
+where
+	L: WalletLCProvider<'a, C, K>,
+	C: NodeClient + 'a,
+	K: Keychain + 'a,
+{
+	wallet_lock!(wallet_inst, w);
+	let active = w.parent_key_id();
+	let k = w.keychain(keychain_mask)?;
+	for acct in w.acct_path_iter()?.filter(|acct| acct.path != active) {
+		let sec_addr_key = address::address_from_derivation_path(&k, &acct.path, 0)?;
+		let dec_key = DalekSecretKey::from_bytes(&sec_addr_key.0);
+		let mut candidate = slatepack.clone();
+		if candidate.try_decrypt_payload(Some(&dec_key)).is_ok() {
+			return Ok(Some(acct.label));
+		}
+	}
+	Ok(None)
+}
+
 /// Create a slatepack message from the given slate
 pub fn create_slatepack_message<'a, L, C, K>(
 	wallet_inst: Arc<Mutex<Box<dyn WalletInst<'a, L, C, K>>>>,
@@ -217,11 +242,24 @@ where
 			};
 			return packer.get_slate(&slatepack);
 		}
-		Err(Error::SlatepackDecryption(
+		// maybe it's for another account
-			"Could not decrypt slatepack with any provided index on the address derivation path"
+		let packer = Slatepacker::new(SlatepackerArgs {
-				.to_owned(),
+			sender: None,
-		)
+			recipients: vec![],
-		.into())
+			dec_key: None,
+		});
+		let encrypted = packer.deser_slatepack(slatepack.as_bytes(), false)?;
+		let msg = match slatepack_account(wallet_inst, keychain_mask, &encrypted)? {
+			Some(label) => format!(
+				"it is encrypted for account '{}' of this wallet, make it the active account",
+				label
+			),
+			None => {
+				"Could not decrypt slatepack with any provided index on the address derivation path"
+					.to_owned()
+			}
+		};
+		Err(Error::SlatepackDecryption(msg))
 	}
 }
 

libwallet/src/contract/utils.rs

@@ -240,6 +240,8 @@ where
 		// field to report whether the transaction data is held.
 		tx_log_entry.stored_tx = Some(format!("{}.grintx", slate.id));
 	}
+	// as update_tx_slate_state does
+	tx_log_entry.tx_slate_state = Some(slate.state.clone());
 	// If we added outputs in this step, we have to create OutputData here because 'batch'
 	// takes the mutable ref and we can no longer call calc_commit_for_cache for output
 	let added_outputs = if !step_added_outputs {

tests/cmd_line_contract.rs

@@ -210,6 +210,62 @@ fn contract_command_test_impl(test_dir: &str) -> Result<(), grin_wallet_controll
 	];
 	execute_command(&app, test_dir, "wallet2", &client2, arg_vec)?;
 
+	// slatepack for another account needs --account
+	let arg_vec = vec!["grin-wallet", "-p", "password1", "account", "-c", "second"];
+	execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
+	let mut second = None;
+	grin_wallet_controller::controller::owner_single_use(
+		wallet1.clone(),
+		mask1,
+		std::path::PathBuf::from(test_dir),
+		|api, m| {
+			api.set_active_account(m, "second")?;
+			second = Some(api.get_slatepack_address(m, 0)?.to_string());
+			api.set_active_account(m, "default")?;
+			Ok(())
+		},
+	)?;
+	let second = second.unwrap();
+	let arg_vec = vec![
+		"grin-wallet",
+		"-p",
+		"password2",
+		"contract",
+		"new",
+		"--receive",
+		"1",
+		"--no-payjoin",
+		"--min_conf",
+		"1",
+		"--encrypt-for",
+		&second,
+	];
+	execute_command(&app, test_dir, "wallet2", &client2, arg_vec)?;
+	let file_name = only_slatepack(test_dir, "wallet2");
+	let arg_vec = vec![
+		"grin-wallet",
+		"-p",
+		"password1",
+		"contract",
+		"view",
+		"-i",
+		&file_name,
+	];
+	let err = execute_command(&app, test_dir, "wallet1", &client1, arg_vec).unwrap_err();
+	assert!(err.to_string().contains("account 'second'"), "{}", err);
+	let arg_vec = vec![
+		"grin-wallet",
+		"-p",
+		"password1",
+		"-a",
+		"second",
+		"contract",
+		"view",
+		"-i",
+		&file_name,
+	];
+	execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
+
 	// A file that isn't there is reported, not panicked on
 	let arg_vec = vec![
 		"grin-wallet",