Stricter types for multisig payloads (#2658)

engine/src/multisig/client/ceremony_manager.rs

@@ -38,8 +38,6 @@ use client::common::{
 	broadcast::BroadcastStage, CeremonyCommon, CeremonyFailureReason, KeygenResultInfo,
 };
 
-use crate::multisig::SigningPayload;
-
 use super::{
 	common::{CeremonyStage, KeygenStageName, PreProcessStageDataCheck, SigningStageName},
 	keygen::{HashCommitments1, HashContext, KeygenData},
@@ -93,11 +91,11 @@ pub struct SigningCeremony<C> {
 	_phantom: PhantomData<C>,
 }
 
-impl<Crypto: CryptoScheme> CeremonyTrait for SigningCeremony<Crypto> {
-	type Crypto = Crypto;
-	type Data = SigningData<<Crypto as CryptoScheme>::Point>;
-	type Request = CeremonyRequest<Crypto>;
-	type Output = <Crypto as CryptoScheme>::Signature;
+impl<C: CryptoScheme> CeremonyTrait for SigningCeremony<C> {
+	type Crypto = C;
+	type Data = SigningData<<C as CryptoScheme>::Point>;
+	type Request = CeremonyRequest<C>;
+	type Output = <C as CryptoScheme>::Signature;
 	type FailureReason = SigningFailureReason;
 	type CeremonyStageName = SigningStageName;
 }
@@ -128,7 +126,7 @@ pub fn prepare_signing_request<Crypto: CryptoScheme>(
 	own_account_id: &AccountId,
 	signers: BTreeSet<AccountId>,
 	key_info: KeygenResultInfo<Crypto::Point>,
-	payload: SigningPayload,
+	payload: Crypto::SigningPayload,
 	outgoing_p2p_message_sender: &UnboundedSender<OutgoingMultisigStageMessages>,
 	rng: Rng,
 	logger: &slog::Logger,
@@ -398,7 +396,7 @@ impl<C: CryptoScheme> CeremonyManager<C> {
 		&mut self,
 		ceremony_id: CeremonyId,
 		signers: BTreeSet<AccountId>,
-		payload: SigningPayload,
+		payload: C::SigningPayload,
 		key_info: KeygenResultInfo<C::Point>,
 		rng: Rng,
 		result_sender: CeremonyResultSender<SigningCeremony<C>>,
@@ -501,7 +499,7 @@ impl<C: CryptoScheme> CeremonyManager<C> {
 
 	fn single_party_signing(
 		&self,
-		payload: SigningPayload,
+		payload: C::SigningPayload,
 		keygen_result_info: KeygenResultInfo<C::Point>,
 		mut rng: Rng,
 	) -> C::Signature {

engine/src/multisig/client/ceremony_manager/tests.rs

@@ -22,7 +22,6 @@ use crate::{
 		},
 		crypto::{CryptoScheme, Rng},
 		eth::{EthSchnorrSignature, EthSigning},
-		tests::fixtures::SIGNING_PAYLOAD,
 	},
 	p2p::OutgoingMultisigStageMessages,
 	task_scope::task_scope,
@@ -50,7 +49,7 @@ async fn run_on_request_to_sign<C: CryptoScheme>(
 			ceremony_manager.on_request_to_sign(
 				ceremony_id,
 				participants,
-				SIGNING_PAYLOAD.clone(),
+				C::signing_payload_for_test(),
 				get_key_data_for_test::<C>(BTreeSet::from_iter(ACCOUNT_IDS.iter().cloned())),
 				Rng::from_seed(DEFAULT_SIGNING_SEED),
 				result_sender,
@@ -93,7 +92,7 @@ fn send_signing_request(
 		ceremony_id,
 		details: Some(CeremonyRequestDetails::Sign(SigningRequestDetails::<EthSigning> {
 			participants,
-			payload: SIGNING_PAYLOAD.clone(),
+			payload: EthSigning::signing_payload_for_test(),
 			keygen_result_info: get_key_data_for_test::<EthSigning>(BTreeSet::from_iter(
 				ACCOUNT_IDS.iter().cloned(),
 			)),

engine/src/multisig/client/ceremony_runner/tests.rs

@@ -17,7 +17,6 @@ use crate::{
 		},
 		crypto::CryptoScheme,
 		eth::{EthSigning, Point},
-		tests::fixtures::SIGNING_PAYLOAD,
 		Rng,
 	},
 	p2p::OutgoingMultisigStageMessages,
@@ -135,7 +134,7 @@ async fn should_delay_stage_1_message_while_unauthorised() {
 		&our_account_id.clone(),
 		participants.clone(),
 		get_key_data_for_test::<EthSigning>(participants),
-		SIGNING_PAYLOAD.clone(),
+		EthSigning::signing_payload_for_test(),
 		&outgoing_p2p_sender,
 		Rng::from_seed(DEFAULT_SIGNING_SEED),
 		&new_test_logger(),
@@ -224,7 +223,7 @@ async fn gen_stage_1_signing_state(
 		&our_account_id.clone(),
 		BTreeSet::from_iter(participants.clone()),
 		get_key_data_for_test::<EthSigning>(BTreeSet::from_iter(participants)),
-		SIGNING_PAYLOAD.clone(),
+		EthSigning::signing_payload_for_test(),
 		&outgoing_p2p_sender,
 		Rng::from_seed(DEFAULT_SIGNING_SEED),
 		&logger,
@@ -335,7 +334,7 @@ async fn should_timeout_authorised_ceremony() {
 			&ACCOUNT_IDS[0],
 			BTreeSet::from_iter(ACCOUNT_IDS.iter().cloned()),
 			get_key_data_for_test::<EthSigning>(BTreeSet::from_iter(ACCOUNT_IDS.iter().cloned())),
-			SIGNING_PAYLOAD.clone(),
+			EthSigning::signing_payload_for_test(),
 			&outgoing_p2p_sender,
 			Rng::from_seed(DEFAULT_SIGNING_SEED),
 			&new_test_logger(),

engine/src/multisig/client/helpers.rs

@@ -35,8 +35,8 @@ use crate::{
 			keygen::{generate_key_data, HashComm1, HashContext, VerifyHashCommitmentsBroadcast2},
 			signing, KeygenResultInfo, PartyIdxMapping, ThresholdParameters,
 		},
-		crypto::{ECPoint, Rng, Verifiable},
-		CryptoScheme, KeyId, SigningPayload,
+		crypto::{ECPoint, Rng},
+		CryptoScheme, KeyId,
 	},
 	p2p::OutgoingMultisigStageMessages,
 };
@@ -51,7 +51,6 @@ use crate::{
 		// This determines which crypto scheme will be used in tests
 		// (we make arbitrary choice to use eth)
 		crypto::eth::{EthSigning, Point},
-		tests::fixtures::SIGNING_PAYLOAD,
 	},
 	testing::expect_recv_with_timeout,
 };
@@ -117,7 +116,7 @@ pub struct SigningCeremonyDetails<C: CryptoScheme> {
 	pub rng: Rng,
 	pub ceremony_id: CeremonyId,
 	pub signers: BTreeSet<AccountId>,
-	pub payload: SigningPayload,
+	pub payload: C::SigningPayload,
 	pub keygen_result_info: KeygenResultInfo<C::Point>,
 }
 
@@ -623,7 +622,7 @@ impl KeygenCeremonyRunner {
 pub struct SigningCeremonyRunnerData<C: CryptoScheme> {
 	pub key_id: KeyId,
 	pub key_data: HashMap<AccountId, KeygenResultInfo<C::Point>>,
-	pub payload: SigningPayload,
+	pub payload: C::SigningPayload,
 }
 pub type SigningCeremonyRunner<C> =
 	CeremonyTestRunner<SigningCeremonyRunnerData<C>, SigningCeremony<C>>;
@@ -641,9 +640,12 @@ impl<C: CryptoScheme> CeremonyRunnerStrategy for SigningCeremonyRunner<C> {
 		let signature = all_same(outputs.into_iter().map(|(_, signature)| signature))
 			.expect("Signatures don't match");
 
-		signature
-			.verify(&self.ceremony_runner_data.key_id, &SIGNING_PAYLOAD)
-			.expect("Should be valid signature");
+		C::verify_signature(
+			&signature,
+			&self.ceremony_runner_data.key_id,
+			&C::signing_payload_for_test(),
+		)
+		.expect("Should be valid signature");
 
 		signature
 	}
@@ -665,7 +667,7 @@ impl<C: CryptoScheme> SigningCeremonyRunner<C> {
 		ceremony_id: CeremonyId,
 		key_id: KeyId,
 		key_data: HashMap<AccountId, KeygenResultInfo<C::Point>>,
-		payload: SigningPayload,
+		payload: C::SigningPayload,
 		rng: Rng,
 	) -> Self {
 		Self::inner_new(
@@ -681,7 +683,7 @@ impl<C: CryptoScheme> SigningCeremonyRunner<C> {
 		ceremony_id: CeremonyId,
 		key_id: KeyId,
 		key_data: HashMap<AccountId, KeygenResultInfo<C::Point>>,
-		payload: SigningPayload,
+		payload: C::SigningPayload,
 		rng: Rng,
 	) -> (Self, HashMap<AccountId, Node<SigningCeremony<C>>>) {
 		let nodes_len = nodes.len();
@@ -726,7 +728,7 @@ pub async fn new_signing_ceremony<C: CryptoScheme>(
 		DEFAULT_SIGNING_CEREMONY_ID,
 		key_id,
 		key_data,
-		SIGNING_PAYLOAD.clone(),
+		C::signing_payload_for_test(),
 		Rng::from_seed(DEFAULT_SIGNING_SEED),
 	)
 }

engine/src/multisig/client/keygen/tests.rs

@@ -18,6 +18,7 @@ use crate::multisig::{
 		utils::PartyIdxMapping,
 	},
 	crypto::Rng,
+	CryptoScheme,
 };
 
 use crate::multisig::crypto::eth::Point;
@@ -886,7 +887,7 @@ mod timeout {
 
 #[tokio::test]
 async fn genesis_keys_can_sign() {
-	use crate::multisig::{crypto::eth::EthSigning, tests::fixtures::SIGNING_PAYLOAD};
+	use crate::multisig::crypto::eth::EthSigning;
 
 	let account_ids: BTreeSet<_> = [1, 2, 3, 4].iter().map(|i| AccountId::new([*i; 32])).collect();
 
@@ -900,7 +901,7 @@ async fn genesis_keys_can_sign() {
 			DEFAULT_SIGNING_CEREMONY_ID,
 			key_id.clone(),
 			key_data.clone(),
-			SIGNING_PAYLOAD.clone(),
+			EthSigning::signing_payload_for_test(),
 			Rng::from_entropy(),
 		);
 	standard_signing(&mut signing_ceremony).await;

engine/src/multisig/client/mod.rs

@@ -58,7 +58,7 @@ use self::{
 
 use super::{
 	crypto::{CryptoScheme, ECPoint},
-	Rng, SigningPayload,
+	Rng,
 };
 
 #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)]
@@ -123,7 +123,7 @@ pub trait MultisigClientApi<C: CryptoScheme> {
 		ceremony_id: CeremonyId,
 		key_id: KeyId,
 		signers: BTreeSet<AccountId>,
-		payload: SigningPayload,
+		payload: C::SigningPayload,
 	) -> BoxFuture<'_, Result<C::Signature, (BTreeSet<AccountId>, SigningFailureReason)>>;
 
 	fn update_latest_ceremony_id(&self, ceremony_id: CeremonyId);
@@ -154,7 +154,7 @@ where
 	C: CryptoScheme,
 {
 	pub participants: BTreeSet<AccountId>,
-	pub payload: SigningPayload,
+	pub payload: C::SigningPayload,
 	pub keygen_result_info: KeygenResultInfo<<C as CryptoScheme>::Point>,
 	pub rng: Rng,
 	pub result_sender: CeremonyResultSender<SigningCeremony<C>>,
@@ -252,14 +252,14 @@ impl<C: CryptoScheme> MultisigClientApi<C> for MultisigClient<C> {
 		ceremony_id: CeremonyId,
 		key_id: KeyId,
 		signers: BTreeSet<AccountId>,
-		payload: SigningPayload,
+		payload: C::SigningPayload,
 	) -> BoxFuture<'_, Result<C::Signature, (BTreeSet<AccountId>, SigningFailureReason)>> {
 		assert!(signers.contains(&self.my_account_id));
 
 		slog::debug!(
 			self.logger,
 			"Received a request to sign";
-			"message_hash" => payload.to_string(),
+			"payload" => payload.to_string(),
 			"signers" => format_iterator(&signers).to_string(),
 			CEREMONY_ID_KEY => ceremony_id
 		);

engine/src/multisig/client/multisig_client_tests.rs

@@ -17,7 +17,7 @@ use crate::{
 			CeremonyRequestDetails,
 		},
 		eth::EthSigning,
-		KeyId, PersistentKeyDB, SigningPayload,
+		KeyId, PersistentKeyDB,
 	},
 	testing::{assert_future_can_complete, new_temp_directory_with_nonexistent_file},
 };
@@ -51,7 +51,7 @@ async fn should_ignore_rts_for_unknown_key() {
 		DEFAULT_SIGNING_CEREMONY_ID,
 		key_id,
 		BTreeSet::from_iter(ACCOUNT_IDS.iter().cloned()),
-		SigningPayload(vec![0; 32]),
+		EthSigning::signing_payload_for_test(),
 	);
 
 	// Check sign request fails immediately with "unknown key" error

engine/src/multisig/client/signing.rs

@@ -7,7 +7,7 @@ mod tests;
 
 use std::sync::Arc;
 
-use crate::multisig::{crypto::ECPoint, SigningPayload};
+use crate::multisig::CryptoScheme;
 
 use super::common::KeygenResult;
 
@@ -27,7 +27,7 @@ pub use signing_detail::get_lagrange_coeff;
 
 /// Data common for signing stages
 #[derive(Clone)]
-pub struct SigningStateCommonInfo<P: ECPoint> {
-	pub payload: SigningPayload,
-	pub key: Arc<KeygenResult<P>>,
+pub struct SigningStateCommonInfo<C: CryptoScheme> {
+	pub payload: C::SigningPayload,
+	pub key: Arc<KeygenResult<C::Point>>,
 }