diff --git a/libwallet/src/contract/context.rs b/libwallet/src/contract/context.rs index 0776407..3c45224 100644 --- a/libwallet/src/contract/context.rs +++ b/libwallet/src/contract/context.rs @@ -95,11 +95,11 @@ where // Make sure `my_fee < net_change` holds for the receiver. This can't be true for a self-spend, because nobody // has a net_change > 0 which makes a self-spend ok to be a net negative when fees are included. if net_change > 0 && my_fee.fee() > net_change.abs() as u64 { - panic!( + return Err(Error::GenericError(format!( "My contribution as a receiver would be net negative. my_fee: {}, net_change: {}", my_fee.fee(), net_change - ); + ))); } // Add my share of fee contribution to the slate fees slate.fee_fields = FeeFields::new(0, slate.fee_fields.fee() + my_fee.fee())?; diff --git a/libwallet/src/contract/selection.rs b/libwallet/src/contract/selection.rs index 6b98afe..0d15e4b 100644 --- a/libwallet/src/contract/selection.rs +++ b/libwallet/src/contract/selection.rs @@ -72,7 +72,7 @@ where inputs.clone().iter().map(|out| out.value).sum::(), fee.fee(), setup_args, - ); + )?; Ok((inputs, output_amounts, fee)) } @@ -258,7 +258,7 @@ fn build_output_amount_list( my_input_sum: u64, my_fee_cost: u64, setup_args: &ContractSetupArgsAPI, -) -> Vec { +) -> Result, Error> { let expected_net_change = setup_args.net_change.unwrap(); let mut my_output_amounts = setup_args.selection_args.output_amounts(); let custom_outputs_sum = my_output_amounts.iter().sum::(); @@ -269,10 +269,10 @@ fn build_output_amount_list( (my_input_sum - custom_outputs_sum) as i64 + expected_net_change - my_fee_cost as i64; // TODO: Check if it's even possible for change output to be negative (it shouldn't be if the equation is correct) if my_change_output_amount < 0 { - panic!( - "contract::selection::build_output_amount_list => ERROR: This should never happen!!! Values: my_input_sum: {}, expected_net_change: {}, my_fee_cost: {}", + return Err(Error::GenericError(format!( + "contract::selection::build_output_amount_list => negative change output. Values: my_input_sum: {}, expected_net_change: {}, my_fee_cost: {}", my_input_sum as i64, expected_net_change, my_fee_cost as i64 - ); + ))); } // Add our change/receiver output (which can be a zero-value output) to the list of outputs my_output_amounts.push(my_change_output_amount as u64); @@ -280,7 +280,7 @@ fn build_output_amount_list( "contract::selection::build_output_amount_list => inputs sum: {}, my_output_amounts:{:#?}", my_input_sum, my_output_amounts ); - my_output_amounts + Ok(my_output_amounts) } /// Compares the output selection args provided at call with those from Context and checks whether they conflict @@ -292,7 +292,10 @@ pub fn verify_selection_consistency( // default or exactly the same strategy we defined when doing the setup phase. // TODO: Test that this works. Perhaps we'd have to define how to compare the two? if cur_args != ctx_args && cur_args != &OutputSelectionArgs::default() { - panic!("Can't define selection args now because we've already done the setup phase. ctx_selection_args:{:#?}, cur_selection_args:{:#?}", ctx_args, cur_args); + return Err(Error::GenericError(format!( + "Can't define selection args now because we've already done the setup phase. ctx_selection_args:{:#?}, cur_selection_args:{:#?}", + ctx_args, cur_args + ))); } // NOTE: The logic above isn't perfect. This is because the user could define arguments that are the default. In this case // we'd simply silently use the arguments provided in the setup phase. This could be confusing for the user. diff --git a/libwallet/src/contract/utils.rs b/libwallet/src/contract/utils.rs index e5007dc..2499689 100644 --- a/libwallet/src/contract/utils.rs +++ b/libwallet/src/contract/utils.rs @@ -82,10 +82,7 @@ where tx_log_entry.num_inputs = context.input_ids.len(); tx_log_entry.fee = context.fee; // Set kernel information - match slate.calc_excess(keychain.secp()) { - Ok(e) => tx_log_entry.kernel_excess = Some(e), - Err(_) => panic!("We can't update tx log entry. Excess could not be computed."), - }; + tx_log_entry.kernel_excess = Some(slate.calc_excess(keychain.secp())?); tx_log_entry.kernel_lookup_min_height = Some(current_height); // If we're sending and there's payment proof info in the slate added by recipient, store as well @@ -151,10 +148,10 @@ where match expected_net_change { Some(args_net_change) => { if ctx.get_net_change() != args_net_change { - panic!( + return Err(Error::GenericError(format!( "Expected net change mismatch! Context.net_change: {}, setup_args.net_change: {}", ctx.get_net_change(), args_net_change - ); + ))); } } None => (), @@ -349,18 +346,18 @@ pub fn verify_setup_args_consistency( ) -> Result<(), Error> { // Compare net_change if ctx_setup_args.net_change.unwrap() != cur_setup_args.net_change.unwrap() { - panic!( + return Err(Error::GenericError(format!( "Inconsistent net change. Ctx net_change:{}, Current net_change: {}", ctx_setup_args.net_change.unwrap(), cur_setup_args.net_change.unwrap() - ); + ))); } // Compare num_participants if ctx_setup_args.num_participants != cur_setup_args.num_participants { - panic!( + return Err(Error::GenericError(format!( "Inconsistent num_participants. Ctx num_participants:{}, Current num_participants: {}", ctx_setup_args.num_participants, cur_setup_args.num_participants - ); + ))); } // TODO: Should we verify add_outputs? // TODO: verify that the parent_key_id is consistent, perhaps even with the active_account set?