From 5569d38df2c26a7317eca8a7c0c24e36d1f72672 Mon Sep 17 00:00:00 2001 From: jwinterm Date: Sun, 7 Jun 2026 17:16:44 -0400 Subject: [PATCH] Contracts: test selection-args consistency and fee splitting Adds the two regression tests requested in review: - verify_selection_consistency: same/default args are accepted; changed non-default args after setup are rejected. - my_fee_contribution: a single participant pays the whole kernel cost, two participants each pay ceil(kernel/2), and the shares cover the full cost. Also resolves the related TODOs (selection.rs "test that this works", utils.rs fee-encoding compatibility note). Addresses PR #754 review (selection.rs, utils.rs). --- libwallet/src/contract/selection.rs | 20 +++++++++++++++++- libwallet/src/contract/utils.rs | 32 +++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/libwallet/src/contract/selection.rs b/libwallet/src/contract/selection.rs index d86903b..9207057 100644 --- a/libwallet/src/contract/selection.rs +++ b/libwallet/src/contract/selection.rs @@ -289,7 +289,6 @@ pub fn verify_selection_consistency( ) -> Result<(), Error> { // We can't define a selection strategy if we've already done the setup phase. We only allow to pass either the // 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() { 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:{:#?}", @@ -576,6 +575,25 @@ mod tests { ); } + #[test] + fn selection_args_consistency() { + // Selection args agreed at the setup phase (stored in the Context). + let ctx_args = OutputSelectionArgs { + use_inputs: Some("any".to_string()), + ..Default::default() + }; + // Re-supplying the same args is fine. + assert!(verify_selection_consistency(&ctx_args, &ctx_args).is_ok()); + // Supplying defaults later is fine (we fall back to the setup-phase args). + assert!(verify_selection_consistency(&ctx_args, &OutputSelectionArgs::default()).is_ok()); + // Supplying different, non-default args after setup is rejected. + let changed = OutputSelectionArgs { + use_inputs: Some("0123456789".to_string()), + ..Default::default() + }; + assert!(verify_selection_consistency(&ctx_args, &changed).is_err()); + } + /* Tests to add: diff --git a/libwallet/src/contract/utils.rs b/libwallet/src/contract/utils.rs index 73846e1..9a80d5c 100644 --- a/libwallet/src/contract/utils.rs +++ b/libwallet/src/contract/utils.rs @@ -324,8 +324,9 @@ pub fn my_fee_contribution( let my_kernel_cost = (kernel_cost as f64 / (num_participants as f64)).ceil(); fee += my_kernel_cost as u64; - // Add my fee contribution to the slate total fee. - // TODO: Does this break compatibility with existing slates? + // Add my fee contribution to the slate total fee. Uses the standard FeeFields + // encoding; contracts rely on every participant applying the same + // 1/num_participants split so the contributions together cover the kernel cost. let my_fee_fields = FeeFields::new(0, fee)?; Ok(my_fee_fields) } @@ -408,3 +409,30 @@ where }; Ok(parent_key_id) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn fee_contribution_kernel_split() { + let (n_inputs, n_outputs, n_kernels) = (1, 1, 1); + let base = tx_fee(n_inputs, n_outputs, 0); + let kernel_cost = tx_fee(0, 0, n_kernels); + + // A single participant (self-spend) pays the whole kernel cost. + let solo = my_fee_contribution(n_inputs, n_outputs, n_kernels, 1) + .unwrap() + .fee(); + assert_eq!(solo, base + kernel_cost); + + // Two participants each pay ceil(kernel_cost / 2) ... + let half = my_fee_contribution(n_inputs, n_outputs, n_kernels, 2) + .unwrap() + .fee(); + let my_share = ((kernel_cost as f64) / 2.0).ceil() as u64; + assert_eq!(half, base + my_share); + // ... and applying the same split, the participants together cover the kernel cost. + assert!(2 * my_share >= kernel_cost); + } +}