From 93881dfb352222fa57e4d2dfeef53da610fb057a Mon Sep 17 00:00:00 2001 From: wiesche Date: Fri, 4 Sep 2026 21:35:36 +0200 Subject: [PATCH] validate contract participant count --- controller/tests/contract_srs.rs | 79 ++++++++++++++++++++++-- doc/contracts.md | 3 +- libwallet/src/contract/actions/README.md | 1 - libwallet/src/contract/actions/new.rs | 1 + libwallet/src/contract/actions/setup.rs | 7 +++ libwallet/src/contract/actions/sign.rs | 1 + libwallet/src/contract/actions/view.rs | 8 +-- libwallet/src/contract/context.rs | 12 +--- libwallet/src/contract/utils.rs | 31 ++++++++++ 9 files changed, 118 insertions(+), 25 deletions(-) diff --git a/controller/tests/contract_srs.rs b/controller/tests/contract_srs.rs index c1584aa..076c8ce 100644 --- a/controller/tests/contract_srs.rs +++ b/controller/tests/contract_srs.rs @@ -49,6 +49,17 @@ fn reject_contract_slate( )) } +fn assert_participant_error(err: libwallet::Error, count: u8) { + assert!(matches!( + err, + libwallet::Error::GenericError(ref msg) + if msg == &format!( + "Unsupported num_participants: {} (expected 1 or 2)", + count + ) + )); +} + /// contract SRS flow fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { // create two wallets and mine 4 blocks in each (we want both to have balance to get a payjoin) @@ -68,7 +79,7 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> PathBuf::from(test_dir), |api, m| { // Send wallet inititates a standard transaction with --send=5 - let args = &mut ContractNewArgsAPI { + let mut args = ContractNewArgsAPI { setup_args: ContractSetupArgsAPI { selection_args: common::contract_selection_args(), net_change: Some(-5_000_000_000), @@ -76,7 +87,16 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> }, ..Default::default() }; - slate = api.contract_new(m, args)?; + let progress = common::wallet_progress(api, m)?; + for count in [0, 3] { + args.setup_args.num_participants = count; + let err = api.contract_new(m, &mut args).unwrap_err(); + assert_participant_error(err, count); + } + assert_eq!(common::wallet_progress(api, m)?, progress); + + args.setup_args.num_participants = 2; + slate = api.contract_new(m, &mut args)?; Ok(()) }, )?; @@ -101,14 +121,40 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> PathBuf::from(test_dir), |api, m| { let progress = common::wallet_progress(api, m)?; - let mut invalid = slate.clone(); - invalid.participant_data[0].part_sig = - Some(Signature::from_raw_data(&[0; 64]).unwrap()); let setup_args = ContractSetupArgsAPI { selection_args: common::contract_selection_args(), net_change: Some(-5_000_000_000), ..Default::default() }; + for count in [0, 3] { + let mut invalid = slate.clone(); + invalid.num_participants = count; + let err = { + let mut owner = api.wallet_inst.lock(); + let wallet = owner.lc_provider()?.wallet_inst()?; + libwallet::contract::setup(wallet, m, &invalid, &setup_args).unwrap_err() + }; + assert_participant_error(err, count); + } + assert_eq!(common::wallet_progress(api, m)?, progress); + + let mut mismatched = setup_args.clone(); + mismatched.num_participants = 1; + let err = { + let mut owner = api.wallet_inst.lock(); + let wallet = owner.lc_provider()?.wallet_inst()?; + libwallet::contract::setup(wallet, m, &slate, &mismatched).unwrap_err() + }; + assert!(matches!( + err, + libwallet::Error::GenericError(ref msg) + if msg == "Inconsistent num_participants. Slate num_participants:2, Setup num_participants: 1" + )); + assert_eq!(common::wallet_progress(api, m)?, progress); + + let mut invalid = slate.clone(); + invalid.participant_data[0].part_sig = + Some(Signature::from_raw_data(&[0; 64]).unwrap()); let err = { let mut owner = api.wallet_inst.lock(); let wallet = owner.lc_provider()?.wallet_inst()?; @@ -126,6 +172,29 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> PathBuf::from(test_dir), |api, m| { let progress = common::wallet_progress(api, m)?; + for count in [0, 3] { + let mut invalid = slate.clone(); + invalid.num_participants = count; + let err = match api.contract_view(m, &invalid) { + Err(err) => err, + Ok(_) => panic!("invalid participant count accepted"), + }; + assert_participant_error(err, count); + let err = api + .contract_sign( + m, + &invalid, + &ContractSetupArgsAPI { + selection_args: common::contract_selection_args(), + net_change: Some(5_000_000_000), + ..Default::default() + }, + ) + .unwrap_err(); + assert_participant_error(err, count); + } + assert_eq!(common::wallet_progress(api, m)?, progress); + let mut invalid = slate.clone(); invalid.state = SlateState::Unknown; let err = api diff --git a/doc/contracts.md b/doc/contracts.md index 4748c5a..87973df 100644 --- a/doc/contracts.md +++ b/doc/contracts.md @@ -63,7 +63,8 @@ account selected with `--account`. ## Current limitations -* Only one or two participants are supported +* Only one or two participants are supported. Larger contracts remain disabled because of + the [known multi-party attack](https://forum.grin.mw/t/grin-wallet-contract-prototype/9745/18) * Early payment proofs are only implemented for contracts and are only available through the API. Only invoice proofs are supported and they require the experimental Slate V5 format. The full proposal is described in diff --git a/libwallet/src/contract/actions/README.md b/libwallet/src/contract/actions/README.md index bd39455..76ec1a1 100644 --- a/libwallet/src/contract/actions/README.md +++ b/libwallet/src/contract/actions/README.md @@ -39,7 +39,6 @@ Separating side effects until the 'save_step' part would make these functions mu - Reserve funds across open late-locked contracts, or make the possible signing failure clearer to callers. - - Keep contracts at two participants until the known multi-party attack is addressed. - For a payjoin, `--use-inputs any` picks an input for us; naming the commitment would be clearer, for the API as much as the CLI. - `new` has no `--no-setup`. diff --git a/libwallet/src/contract/actions/new.rs b/libwallet/src/contract/actions/new.rs index 25001fd..44982a8 100644 --- a/libwallet/src/contract/actions/new.rs +++ b/libwallet/src/contract/actions/new.rs @@ -38,6 +38,7 @@ where C: NodeClient, K: Keychain, { + contract::utils::verify_num_participants(setup_args.num_participants)?; // Compute state for 'new' let (slate, context) = compute(w, keychain_mask, setup_args, slate_id)?; diff --git a/libwallet/src/contract/actions/setup.rs b/libwallet/src/contract/actions/setup.rs index 452cedf..cfd8656 100644 --- a/libwallet/src/contract/actions/setup.rs +++ b/libwallet/src/contract/actions/setup.rs @@ -35,6 +35,13 @@ where C: NodeClient, K: Keychain, { + contract::utils::verify_num_participants(slate.num_participants)?; + if slate.num_participants != setup_args.num_participants { + return Err(Error::GenericError(format!( + "Inconsistent num_participants. Slate num_participants:{}, Setup num_participants: {}", + slate.num_participants, setup_args.num_participants + ))); + } contract::utils::verify_not_signed(w, slate.id)?; // Compute state for 'setup' let (slate, context) = compute(w, keychain_mask, slate, setup_args)?; diff --git a/libwallet/src/contract/actions/sign.rs b/libwallet/src/contract/actions/sign.rs index fcbfbde..2620d0b 100644 --- a/libwallet/src/contract/actions/sign.rs +++ b/libwallet/src/contract/actions/sign.rs @@ -58,6 +58,7 @@ where C: NodeClient, K: Keychain, { + contract::utils::verify_num_participants(slate.num_participants)?; // Compute if we will add outputs at this step. Only a missing context means we // still need to add them; propagate any other error. let existing_context = match w.get_private_context(keychain_mask, slate.id.as_bytes()) { diff --git a/libwallet/src/contract/actions/view.rs b/libwallet/src/contract/actions/view.rs index a5da2c4..b5a3f59 100644 --- a/libwallet/src/contract/actions/view.rs +++ b/libwallet/src/contract/actions/view.rs @@ -75,13 +75,7 @@ where "Cannot view a slate with an Unknown state".to_string(), )); } - // Mirror the contract setup bound so a tampered slate can't surface a bogus count. - if slate.num_participants < 1 || slate.num_participants > 2 { - return Err(Error::GenericError(format!( - "Unsupported num_participants: {} (expected 1 or 2)", - slate.num_participants - ))); - } + contract::utils::verify_num_participants(slate.num_participants)?; let context = match w.get_private_context(keychain_mask, slate.id.as_bytes()) { Ok(context) => Some(context), Err(Error::NotFoundErr(_)) => None, diff --git a/libwallet/src/contract/context.rs b/libwallet/src/contract/context.rs index e913217..6a52245 100644 --- a/libwallet/src/contract/context.rs +++ b/libwallet/src/contract/context.rs @@ -79,17 +79,7 @@ where K: Keychain, { debug!("Creating a new contract context"); - // Validate num_participants before any fee math (kernel cost is divided by it, so 0 - // would be a divide-by-zero). This is the common setup path for every party (the - // signer's value comes from the slate), so it also guards against a tampered slate. - // Only 1 (self-spend) and 2 (standard) are implemented today; lift this when - // multi-party contracts are supported. - if setup_args.num_participants < 1 || setup_args.num_participants > 2 { - return Err(Error::GenericError(format!( - "Unsupported num_participants: {} (expected 1 or 2)", - setup_args.num_participants - ))); - } + contract_utils::verify_num_participants(setup_args.num_participants)?; // sender should always refresh outputs updater::refresh_outputs(w, keychain_mask, parent_key_id, false)?; diff --git a/libwallet/src/contract/utils.rs b/libwallet/src/contract/utils.rs index 962191c..c18c0b0 100644 --- a/libwallet/src/contract/utils.rs +++ b/libwallet/src/contract/utils.rs @@ -346,6 +346,7 @@ pub fn my_fee_contribution( n_kernels: usize, num_participants: u8, ) -> Result { + verify_num_participants(num_participants)?; // Add our fee costs for our inputs and a single output 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 @@ -362,6 +363,18 @@ pub fn my_fee_contribution( Ok(my_fee_fields) } +/// Contracts need at least one participant for fee splitting +/// More than two remain disabled because of the known multi-party attack +pub(super) fn verify_num_participants(num_participants: u8) -> Result<(), Error> { + if !(1..=2).contains(&num_participants) { + return Err(Error::GenericError(format!( + "Unsupported num_participants: {} (expected 1 or 2)", + num_participants + ))); + } + Ok(()) +} + /// Whether a transaction log entry shows that this wallet signed the slate. pub(super) fn is_signed_tx(tx: &TxLogEntry, slate_id: Uuid) -> bool { tx.tx_slate_id == Some(slate_id) && tx.kernel_excess.is_some() @@ -469,4 +482,22 @@ mod tests { // ... and applying the same split, the participants together cover the kernel cost. assert!(2 * my_share >= kernel_cost); } + + #[test] + fn participant_count() { + assert!(verify_num_participants(1).is_ok()); + assert!(verify_num_participants(2).is_ok()); + for count in [0, 3] { + let err = verify_num_participants(count).unwrap_err(); + assert!(matches!( + err, + Error::GenericError(ref msg) + if msg == &format!( + "Unsupported num_participants: {} (expected 1 or 2)", + count + ) + )); + assert!(my_fee_contribution(1, 1, 1, count).is_err()); + } + } }