validate contract participant count

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

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

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`.

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)?;
 

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)?;

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()) {

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,

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)?;
 

libwallet/src/contract/utils.rs

@@ -346,6 +346,7 @@ pub fn my_fee_contribution(
 	n_kernels: usize,
 	num_participants: u8,
 ) -> Result<FeeFields, Error> {
+	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());
+		}
+	}
 }