diff --git a/libwallet/src/contract/selection.rs b/libwallet/src/contract/selection.rs index 9207057..24b887d 100644 --- a/libwallet/src/contract/selection.rs +++ b/libwallet/src/contract/selection.rs @@ -266,7 +266,7 @@ fn build_output_amount_list( // TODO: check bounds when casting. let my_change_output_amount = (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) + // A negative change output would mean the selection math is off; reject it explicitly. if my_change_output_amount < 0 { return Err(Error::GenericError(format!( "contract::selection::build_output_amount_list => negative change output. Values: my_input_sum: {}, expected_net_change: {}, my_fee_cost: {}", diff --git a/libwallet/src/contract/types.rs b/libwallet/src/contract/types.rs index e6c847e..48c491f 100644 --- a/libwallet/src/contract/types.rs +++ b/libwallet/src/contract/types.rs @@ -76,7 +76,6 @@ impl OutputSelectionArgs { .unwrap_or(0) } - // TODO: make sure to validate this: if custom outputs are specified, it has to be a payjoin. } impl Default for OutputSelectionArgs { diff --git a/libwallet/src/contract/utils.rs b/libwallet/src/contract/utils.rs index e6a8f8e..1b7ad7e 100644 --- a/libwallet/src/contract/utils.rs +++ b/libwallet/src/contract/utils.rs @@ -319,7 +319,8 @@ pub fn my_fee_contribution( let mut fee = tx_fee(n_inputs, n_outputs, 0); // Add out fee costs for kernel. We pay 1/num_participants of a kernel cost let kernel_cost = tx_fee(0, 0, n_kernels); - // TODO: we slightly overpay. Make sure to cover all the cases + // Round each participant's kernel share up, so the participants together never underpay; + // the overpay is bounded to under one fee unit per participant. let my_kernel_cost = (kernel_cost as f64 / (num_participants as f64)).ceil(); fee += my_kernel_cost as u64; @@ -376,8 +377,9 @@ pub fn verify_setup_args_consistency( 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? + // add_outputs is intentionally forced true at the sign step (late lock), so it is not + // part of this consistency check. parent_key_id is taken from the stored Context, so + // later steps always derive under the contract's account regardless of the active one. // Compare OutputSelectionArgs verify_selection_consistency(