simplify contract helpers

libwallet/src/api_impl/foreign.rs

@@ -303,14 +303,9 @@ where
 {
 	let mut client = w.w2n_client().clone();
 
-	let wd = match proof.witness_data.clone() {
+	let wd = proof.witness_data.as_ref().ok_or_else(|| {
-		Some(w) => w,
+		Error::PaymentProof("Cannot verify early payment proof with no witness data".to_string())
-		None => {
+	})?;
-			return Err(Error::PaymentProof(
-				"Cannot verify early payment proof with no witness data".to_string(),
-			))
-		}
-	};
 
 	let (retrieved_kernel, _) = match client.get_kernel(&wd.kernel_commitment, None, None) {
 		Err(e) => {

libwallet/src/api_impl/owner.rs

@@ -571,51 +571,39 @@ where
 		.as_ref()
 		.and_then(|proof| proof.sender_public_nonce);
 
-	let (mut proof, sender_part_sig) = match tx.payment_proof {
+	let p = tx.payment_proof.ok_or_else(|| {
-		Some(p) => {
+		Error::PaymentProofRetrieval("Transaction does not contain a payment proof".to_owned())
-			if p.receiver_public_nonce.is_none() {
+	})?;
-				return Err(Error::PaymentProofRetrieval(
+	let receiver_public_nonce = p.receiver_public_nonce.ok_or_else(|| {
-					"Early payment proof requires stored receiver public nonce".into(),
+		Error::PaymentProofRetrieval(
-				));
+			"Early payment proof requires stored receiver public nonce".into(),
-			};
+		)
-			if p.receiver_public_excess.is_none() {
+	})?;
-				return Err(Error::PaymentProofRetrieval(
+	let receiver_public_excess = p.receiver_public_excess.ok_or_else(|| {
-					"Early payment proof requires stored receiver public excess".into(),
+		Error::PaymentProofRetrieval(
-				));
+			"Early payment proof requires stored receiver public excess".into(),
-			};
+		)
-			if p.timestamp.is_none() {
+	})?;
-				return Err(Error::PaymentProofRetrieval(
+	let timestamp = p.timestamp.ok_or_else(|| {
-					"Early payment proof requires stored timestamp".into(),
+		Error::PaymentProofRetrieval("Early payment proof requires stored timestamp".into())
-				));
+	})?;
-			};
+	let sender_part_sig = p.sender_part_sig.ok_or_else(|| {
-			if p.sender_part_sig.is_none() {
+		Error::PaymentProofRetrieval(
-				return Err(Error::PaymentProofRetrieval(
+			"Early payment proof requires stored sender partial signature".into(),
-					"Early payment proof requires stored sender partial signature".into(),
+		)
-				));
+	})?;
-			};
+	let mut proof = EarlyPaymentProof {
-
+		proof_type: PaymentProofType::try_from(
-			(
+			p.proof_type.unwrap_or(PaymentProofType::Invoice.as_u8()),
-				EarlyPaymentProof {
+		)?,
-					proof_type: PaymentProofType::try_from(
+		amount,
-						p.proof_type.unwrap_or(PaymentProofType::Invoice.as_u8()),
+		receiver_public_nonce,
-					)?,
+		receiver_public_excess,
-					amount,
+		sender_address: p.sender_address,
-					receiver_public_nonce: p.receiver_public_nonce.unwrap(),
+		timestamp: timestamp.timestamp(),
-					receiver_public_excess: p.receiver_public_excess.unwrap(),
+		memo: p.memo,
-					sender_address: p.sender_address,
+		promise_signature: p.receiver_signature,
-					timestamp: p.timestamp.unwrap().timestamp(),
+		witness_data: None,
-					memo: p.memo,
-					promise_signature: p.receiver_signature,
-					witness_data: None,
-				},
-				p.sender_part_sig.unwrap(),
-			)
-		}
-		None => {
-			return Err(Error::PaymentProofRetrieval(
-				"Transaction does not contain a payment proof".to_owned(),
-			));
-		}
 	};
 
 	// Now to kernel lookup, to fill in the witness data

libwallet/src/backend.rs

@@ -328,7 +328,7 @@ where
 	/// Get an (Optional) tx log entry by parent key id and log id.
 	pub fn get_tx_log_entry_by_id(
 		&self,
-		parent_id: Identifier,
+		parent_id: &Identifier,
 		log_id: u32,
 	) -> Result<Option<TxLogEntry>, Error> {
 		let tx_log_key = to_key_u64(parent_id.to_bytes(), log_id as u64);

libwallet/src/contract/actions/revoke.rs

@@ -55,7 +55,7 @@ where
 	// cannot be built until the input is unlocked.
 	// FUTURE: we may want to boost fees if we notice the original tx in the mempool.
 	let tx_id = args.tx_id;
-	let parent_key_id = utils::parent_key_for(w, args.src_acct_name.as_ref())?;
+	let parent_key_id = utils::parent_key_for(w, args.src_acct_name.as_deref())?;
 
 	// Inputs we contributed to tx_id that are still recoverable. Locked => the original tx
 	// is still active; Unspent => a previous revoke cancelled it but the self-spend did not
@@ -75,7 +75,7 @@ where
 	// it is already a *Cancelled type (and the inputs are Unspent), so we skip straight to
 	// re-spending them.
 	let revoked = w
-		.get_tx_log_entry_by_id(parent_key_id.clone(), tx_id)?
+		.get_tx_log_entry_by_id(&parent_key_id, tx_id)?
 		.ok_or_else(|| Error::NotFoundErr(format!("Transaction {}", tx_id)))?;
 	let revoked_slate_id = revoked.tx_slate_id;
 	let needs_cancel = matches!(

libwallet/src/contract/context.rs

@@ -46,7 +46,7 @@ where
 			// Get data required for creating a context
 			let height = w.w2n_client().get_chain_tip()?.0;
 			let parent_key_id =
-				contract_utils::parent_key_for(w, setup_args.src_acct_name.as_ref())?;
+				contract_utils::parent_key_for(w, setup_args.src_acct_name.as_deref())?;
 			self::create(
 				w,
 				keychain_mask,
@@ -179,14 +179,14 @@ where
 		)));
 	}
 	// Add selected/created inputs/outputs to the context
-	add_inputs_to_ctx(context, &inputs)?;
+	add_inputs_to_ctx(context, &inputs);
 	add_outputs_to_ctx(w, keychain_mask, context, my_output_amounts)?;
 
 	Ok(())
 }
 
 /// Add inputs to Context
-fn add_inputs_to_ctx(context: &mut Context, inputs: &Vec<OutputData>) -> Result<(), Error> {
+fn add_inputs_to_ctx(context: &mut Context, inputs: &[OutputData]) {
 	debug!("contract::utils::add_inputs_to_ctx => adding inputs to context");
 	for input in inputs {
 		context.add_input(&input.key_id, &input.mmr_index, input.value);
@@ -195,8 +195,6 @@ fn add_inputs_to_ctx(context: &mut Context, inputs: &Vec<OutputData>) -> Result<
 			&input.key_id, input.value
 		);
 	}
-
-	Ok(())
 }
 
 /// Add outputs to Context

libwallet/src/contract/selection.rs

@@ -71,10 +71,8 @@ where
 pub fn compute(
 	setup_args: &ContractSetupArgsAPI,
 	committed_fee: Option<FeeFields>,
-	inputs: &mut Vec<OutputData>,
+	inputs: &mut [OutputData],
-) -> Result<(Vec<OutputData>, Vec<u64>, FeeFields), Error>
+) -> Result<(Vec<OutputData>, Vec<u64>, FeeFields), Error> {
-where
-{
 	let (inputs, fee) = select_inputs(setup_args, committed_fee, inputs)?;
 	let input_sum = inputs
 		.iter()
@@ -88,10 +86,8 @@ where
 fn select_inputs(
 	setup_args: &ContractSetupArgsAPI,
 	committed_fee: Option<FeeFields>,
-	inputs: &mut Vec<OutputData>,
+	inputs: &mut [OutputData],
-) -> Result<(Vec<OutputData>, FeeFields), Error>
+) -> Result<(Vec<OutputData>, FeeFields), Error> {
-where
-{
 	// We use 'lhs' and 'rhs' to denote the amounts on the left/right-hand side of the equation.
 	// To simulate receive/payment value we:
 	// - add positive net_change to the 'lhs' for the receiver (to simulate sender's input)
@@ -142,15 +138,16 @@ where
 	}
 	// NOTE: that these are inputs that MUST be selected. We should lock the inputs if they're
 	// required to minimize any potential race conditions.
-	let must_use_list = required_inputs.unwrap_or(vec![]);
+	let must_use_list = required_inputs.unwrap_or_default();
-	if must_use_list.len() > 0 {
+	if !must_use_list.is_empty() {
 		// Sort the inputs first by the ones listed in the use_inputs and then by value
 		inputs.sort_by_key(|out| {
 			(
 				// We have to negate the boolean to prioritize truthy values because
 				// false is 0 and hence would be sorted before truthy entries
-				!(out.commit.is_some()
+				!out.commit
-					&& must_use_list.contains(&&out.commit.as_ref().unwrap()[..])),
+					.as_deref()
+					.is_some_and(|commit| must_use_list.contains(&commit)),
 				out.value,
 			)
 		});
@@ -171,7 +168,7 @@ where
 	// We want to count how many inputs we've picked _so far_. This is used to prevent picking
 	// all 0*H +r*G outputs when we call with min_input_amount=0 and want just a payjoin.
 	let mut n_inputs = 0;
-	let mut must_use_list_cnt: u32 = 0;
+	let mut must_use_list_cnt: usize = 0;
 	// If we have already committed to a fee (context.fee) then set this as our "minimum" fee. The reason we have to
 	// do this is to avoid solving the equation for less than the committed fee. We have to guarantee the inputs we take
 	// are enough to cover the committed fee. At the end of selection, we check that the fees for the selection were not
@@ -193,8 +190,10 @@ where
 	let mut selected_inputs = Vec::new();
 	for out in inputs.iter() {
 		// Take the commitment if it is listed as one of those we MUST take
-		let must_take =
+		let must_take = out
-			out.commit.is_some() && must_use_list.contains(&&out.commit.as_ref().unwrap()[..]);
+			.commit
+			.as_deref()
+			.is_some_and(|commit| must_use_list.contains(&commit));
 		// Compute the fee without this input
 		let fee_without = fee_contribution(
 			n_inputs,
@@ -251,13 +250,15 @@ where
 	}
 
 	// Return an error if the fee computed is larger than the committed fee
-	if committed_fee.is_some() && my_fee.fee() > committed_fee.unwrap().fee() {
+	if let Some(committed_fee) = committed_fee {
-		let msg = format!(
+		if my_fee.fee() > committed_fee.fee() {
-			"Fee computed ({}) is larger than the committed fee ({}); cancel the transaction and retry",
+			let msg = format!(
-			my_fee.fee(),
+				"Fee computed ({}) is larger than the committed fee ({}); cancel the transaction and retry",
-			committed_fee.unwrap().fee()
+				my_fee.fee(),
-		);
+				committed_fee.fee()
-		return Err(Error::Fee(msg).into());
+			);
+			return Err(Error::Fee(msg));
+		}
 	}
 
 	// Check that the inputs we picked are enough to cover all our output amounts and fees
@@ -276,19 +277,18 @@ where
 			available_disp: amount_to_hr_string(total, false),
 			needed,
 			needed_disp: amount_to_hr_string(needed, false),
-		}
+		});
-		.into());
-		// return Err(ErrorKind::GenericError(msg.into()).into());
 	}
 
 	// Assert that all the use_inputs have been selected
-	if must_use_list.len() != must_use_list_cnt as usize {
+	if let Some(use_inputs) = &setup_args.selection_args.use_inputs {
-		let msg = format!(
+		if must_use_list.len() != must_use_list_cnt {
-			"We have not found all the inputs that have been requested. {}, found only: {}",
+			let msg = format!(
-			setup_args.selection_args.use_inputs.as_ref().unwrap(),
+				"We have not found all the inputs that have been requested. {}, found only: {}",
-			must_use_list_cnt
+				use_inputs, must_use_list_cnt
-		);
+			);
-		return Err(Error::GenericError(msg.into()).into());
+			return Err(Error::GenericError(msg));
+		}
 	}
 
 	debug!(
@@ -307,7 +307,7 @@ fn build_output_amount_list(
 	let expected_net_change = setup_args.net_change.ok_or_else(|| {
 		Error::GenericError("Contract requires a net change (--send or --receive)".to_string())
 	})?;
-	let mut my_output_amounts = setup_args.selection_args.output_amounts()?;
+	let mut my_output_amounts = setup_args.selection_args.output_amounts();
 	let custom_outputs_sum = my_output_amounts
 		.iter()
 		.try_fold(0u64, |acc, v| acc.checked_add(*v))

libwallet/src/contract/types.rs

@@ -49,21 +49,14 @@ impl OutputSelectionArgs {
 	}
 	/// Return a list of commitments we must use
 	pub fn required_inputs(&self) -> Option<Vec<&str>> {
-		if self.use_inputs.is_some() {
+		self.use_inputs
-			Some(
+			.as_deref()
-				self.use_inputs.as_ref().unwrap()[..]
+			.map(|inputs| inputs.split(',').filter(|x| *x != "any").collect())
-					.split(",")
-					.filter(|x| *x != "any")
-					.collect(),
-			)
-		} else {
-			None
-		}
 	}
 	/// Returns the output amounts (nanogrin) we have to create. Amounts arrive already
 	/// parsed by the caller (e.g. the CLI), so this is just an accessor.
-	pub fn output_amounts(&self) -> Result<Vec<u64>, Error> {
+	pub fn output_amounts(&self) -> Vec<u64> {
-		Ok(self.make_outputs.clone().unwrap_or_default())
+		self.make_outputs.clone().unwrap_or_default()
 	}
 	/// Returns the sum of our output amounts
 	pub fn sum_output_amounts(&self) -> Result<u64, Error> {

libwallet/src/contract/utils.rs

@@ -33,7 +33,7 @@ pub fn create_tx_log_entry(
 	net_change: i64,
 	parent_key_id: Identifier,
 	log_id: u32,
-) -> Result<TxLogEntry, Error> {
+) -> TxLogEntry {
 	let log_type = if slate.num_participants == 1 {
 		TxLogEntryType::TxSelfSpend
 	} else {
@@ -43,7 +43,7 @@ pub fn create_tx_log_entry(
 			TxLogEntryType::TxSent
 		}
 	};
-	let mut t = TxLogEntry::new(parent_key_id.clone(), log_type, log_id);
+	let mut t = TxLogEntry::new(parent_key_id, log_type, log_id);
 	// stored_tx is set in save_step, once we have signed and the transaction is written.
 
 	t.tx_slate_id = Some(slate.id);
@@ -57,7 +57,7 @@ pub fn create_tx_log_entry(
 		n => Some(n),
 	};
 
-	Ok(t)
+	t
 }
 
 /// Update TxLogEntry with data from the sign step
@@ -158,16 +158,13 @@ pub fn get_net_change(
 		// 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 {
+		if let Some(args_net_change) = expected_net_change {
-			Some(args_net_change) => {
+			if ctx_net_change != args_net_change {
-				if ctx_net_change != args_net_change {
+				return Err(Error::GenericError(format!(
-					return Err(Error::GenericError(format!(
+					"Expected net change mismatch! Context.net_change: {}, setup_args.net_change: {}",
-						"Expected net change mismatch! Context.net_change: {}, setup_args.net_change: {}",
+					ctx_net_change, args_net_change
-						ctx_net_change, args_net_change
+				)));
-					)));
-				}
 			}
-			None => (),
 		}
 		expected_net_change = Some(ctx_net_change);
 	} else {
@@ -176,18 +173,15 @@ pub fn get_net_change(
 
 	// 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.
-	if expected_net_change.is_none() {
+	let expected_net_change = expected_net_change.ok_or_else(|| {
-		return Err(Error::GenericError(
+		Error::GenericError("You did not agree on the expected net difference.".into())
-			"You did not agree on the expected net difference.".into(),
+	})?;
-		)
-		.into());
-	}
 	debug!(
 		"contract::utils::get_net_change => expected_net_change: {}",
-		expected_net_change.unwrap()
+		expected_net_change
 	);
 
-	Ok(expected_net_change.unwrap())
+	Ok(expected_net_change)
 }
 
 /// Lock inputs and store the Context, TxLogEntry and OutputData atomically
@@ -221,15 +215,14 @@ where
 	// We are at step2 if we don't have context.log_id and we have signed the slate
 	let is_step2 = context.log_id.is_none() && is_signed;
 
-	let mut tx_log_entry = {
+	let mut tx_log_entry = match context.log_id {
-		if context.log_id.is_none() {
+		None => {
 			// We create a new entry with log_id=0 and but replace it with the real id before committing
-			create_tx_log_entry(slate, context.get_net_change()?, parent_key_id.clone(), 0)?
+			create_tx_log_entry(slate, context.get_net_change()?, parent_key_id.clone(), 0)
-		} else {
-			let log_id = context.log_id.unwrap();
-			w.get_tx_log_entry_by_id(parent_key_id.clone(), log_id)?
-				.ok_or_else(|| Error::NotFoundErr(format!("Transaction log entry {}", log_id)))?
 		}
+		Some(log_id) => w
+			.get_tx_log_entry_by_id(parent_key_id, log_id)?
+			.ok_or_else(|| Error::NotFoundErr(format!("Transaction log entry {}", log_id)))?,
 	};
 
 	// Update TxLogEntry if we have signed the contract (we have data about the kernel)
@@ -280,11 +273,11 @@ where
 	// Update TxLogEntry
 	if context.log_id.is_none() {
 		// If we just created the TxLogEntry, we have to assign it an id
-		let log_id = batch.next_tx_log_id(&parent_key_id)?;
+		let log_id = batch.next_tx_log_id(parent_key_id)?;
 		tx_log_entry.id = log_id;
 		context.log_id = Some(log_id);
 	}
-	batch.save_tx_log_entry(tx_log_entry.clone(), &parent_key_id)?;
+	batch.save_tx_log_entry(tx_log_entry, parent_key_id)?;
 	// Create OutputData entries and lock inputs if we added outputs at this step
 	if step_added_outputs {
 		// Create an OutputData entry for every created output
@@ -493,16 +486,16 @@ pub fn verify_setup_args_consistency(
 /// active account.
 pub fn parent_key_for<C, K>(
 	w: &mut WalletBackend<C, K>,
-	src_acct_name: Option<&String>,
+	src_acct_name: Option<&str>,
 ) -> Result<Identifier, Error>
 where
 	C: NodeClient,
 	K: Keychain,
 {
 	let parent_key_id = match src_acct_name {
-		Some(d) => match w.get_acct_path(d.clone())? {
+		Some(d) => match w.get_acct_path(d.to_owned())? {
 			Some(p) => p.path,
-			None => return Err(Error::UnknownAccountLabel(d.clone())),
+			None => return Err(Error::UnknownAccountLabel(d.to_owned())),
 		},
 		None => w.parent_key_id(),
 	};
@@ -519,7 +512,7 @@ mod tests {
 		for (net_change, credited, debited) in
 			[(1, 1, 0), (0, 0, 0), (-1, 0, 1), (i64::MIN, 0, 1u64 << 63)]
 		{
-			let entry = create_tx_log_entry(&slate, net_change, Identifier::zero(), 0).unwrap();
+			let entry = create_tx_log_entry(&slate, net_change, Identifier::zero(), 0);
 			assert_eq!(entry.amount_credited, credited);
 			assert_eq!(entry.amount_debited, debited);
 		}

libwallet/src/payment_proof.rs

@@ -328,11 +328,10 @@ impl EarlyPaymentProof {
 		recipient_address: &DalekPublicKey,
 	) -> Result<(), Error> {
 		check_proof_type(&self.proof_type)?;
-		if self.promise_signature.is_none() {
+		let promise_signature = self
-			return Err(Error::PaymentProofValidation(
+			.promise_signature
-				"Missing promise signature".into(),
+			.as_ref()
-			));
+			.ok_or_else(|| Error::PaymentProofValidation("Missing promise signature".into()))?;
-		}
 
 		// Rebuild message
 		let mut sig_data_bin = Vec::new();
@@ -341,7 +340,7 @@ impl EarlyPaymentProof {
 		})?;
 
 		if recipient_address
-			.verify(&sig_data_bin, self.promise_signature.as_ref().unwrap())
+			.verify(&sig_data_bin, promise_signature)
 			.is_err()
 		{
 			return Err(Error::PaymentProof(
@@ -359,13 +358,13 @@ impl EarlyPaymentProof {
 		excess_sig: &Signature,
 		msg: &Message,
 	) -> Result<(), Error> {
-		if self.witness_data.is_none() {
+		let wd = self
-			return Err(Error::PaymentProofValidation("Missing witness data".into()));
+			.witness_data
-		}
+			.as_ref()
+			.ok_or_else(|| Error::PaymentProofValidation("Missing witness data".into()))?;
 
 		self.verify_promise_signature(recipient_address)?;
 
-		let wd = self.witness_data.as_ref().unwrap().clone();
 		{
 			let static_secp = static_secp_instance();
 			let static_secp = static_secp.lock();