From 943f8654e824884b0c3b0419966587a2bddebbf1 Mon Sep 17 00:00:00 2001 From: jferrant Date: Mon, 8 Apr 2024 12:30:31 -0700 Subject: [PATCH] Do not reprocess old round ids (#9) * Do not reprocess old round ids Signed-off-by: Jacinta Ferrant * Add tests to ensure old round ids are ignored Signed-off-by: Jacinta Ferrant --------- Signed-off-by: Jacinta Ferrant --- src/state_machine/coordinator/fire.rs | 91 +++++++++++++++++++++++++- src/state_machine/coordinator/frost.rs | 91 +++++++++++++++++++++++++- 2 files changed, 180 insertions(+), 2 deletions(-) diff --git a/src/state_machine/coordinator/fire.rs b/src/state_machine/coordinator/fire.rs index 71363e9b..05515d1c 100644 --- a/src/state_machine/coordinator/fire.rs +++ b/src/state_machine/coordinator/fire.rs @@ -219,6 +219,10 @@ impl Coordinator { State::Idle => { // Did we receive a coordinator message? if let Message::DkgBegin(dkg_begin) = &packet.msg { + if self.current_dkg_id >= dkg_begin.dkg_id { + // We have already processed this DKG round + return Ok((None, None)); + } // Set the current sign id to one before the current message to ensure // that we start the next round at the correct id. (Do this rather // than overwriting afterwards to ensure logging is accurate) @@ -226,6 +230,10 @@ impl Coordinator { let packet = self.start_dkg_round()?; return Ok((Some(packet), None)); } else if let Message::NonceRequest(nonce_request) = &packet.msg { + if self.current_sign_id >= nonce_request.sign_id { + // We have already processed this sign round + return Ok((None, None)); + } // Set the current sign id to one before the current message to ensure // that we start the next round at the correct id. (Do this rather // than overwriting afterwards to ensure logging is accurate) @@ -1232,7 +1240,7 @@ impl CoordinatorTrait for Coordinator { pub mod test { use crate::{ curve::{point::Point, scalar::Scalar}, - net::{DkgFailure, DkgPrivateShares, Message, Packet}, + net::{DkgBegin, DkgFailure, DkgPrivateShares, Message, NonceRequest, Packet}, state_machine::{ coordinator::{ fire::Coordinator as FireCoordinator, @@ -2375,4 +2383,85 @@ pub mod test { assert_eq!(coordinator.state, State::Idle); } } + + #[test] + fn old_round_ids_are_ignored_v1() { + old_round_ids_are_ignored::(); + } + + #[test] + fn old_round_ids_are_ignored_v2() { + old_round_ids_are_ignored::(); + } + + fn old_round_ids_are_ignored() { + let (mut coordinators, _) = setup::, Signer>(3, 10); + for coordinator in &mut coordinators { + let id: u64 = 10; + let old_id = id.saturating_sub(1); + coordinator.current_dkg_id = id; + coordinator.current_sign_id = id; + // Attempt to start an old DKG round + let (packets, results) = coordinator + .process_inbound_messages(&[Packet { + sig: vec![], + msg: Message::DkgBegin(DkgBegin { dkg_id: old_id }), + }]) + .unwrap(); + assert!(packets.is_empty()); + assert!(results.is_empty()); + assert_eq!(coordinator.state, State::Idle); + assert_eq!(coordinator.current_dkg_id, id); + + // Attempt to start the same DKG round + let (packets, results) = coordinator + .process_inbound_messages(&[Packet { + sig: vec![], + msg: Message::DkgBegin(DkgBegin { dkg_id: id }), + }]) + .unwrap(); + assert!(packets.is_empty()); + assert!(results.is_empty()); + assert_eq!(coordinator.state, State::Idle); + assert_eq!(coordinator.current_dkg_id, id); + + // Attempt to start an old Sign round + let (packets, results) = coordinator + .process_inbound_messages(&[Packet { + sig: vec![], + msg: Message::NonceRequest(NonceRequest { + dkg_id: id, + sign_id: old_id, + message: vec![], + sign_iter_id: id, + is_taproot: false, + merkle_root: None, + }), + }]) + .unwrap(); + assert!(packets.is_empty()); + assert!(results.is_empty()); + assert_eq!(coordinator.state, State::Idle); + assert_eq!(coordinator.current_sign_id, id); + + // Attempt to start the same Sign round + let (packets, results) = coordinator + .process_inbound_messages(&[Packet { + sig: vec![], + msg: Message::NonceRequest(NonceRequest { + dkg_id: id, + sign_id: id, + message: vec![], + sign_iter_id: id, + is_taproot: false, + merkle_root: None, + }), + }]) + .unwrap(); + assert!(packets.is_empty()); + assert!(results.is_empty()); + assert_eq!(coordinator.state, State::Idle); + assert_eq!(coordinator.current_sign_id, id); + } + } } diff --git a/src/state_machine/coordinator/frost.rs b/src/state_machine/coordinator/frost.rs index 27c02c99..9409388d 100644 --- a/src/state_machine/coordinator/frost.rs +++ b/src/state_machine/coordinator/frost.rs @@ -61,6 +61,10 @@ impl Coordinator { State::Idle => { // Did we receive a coordinator message? if let Message::DkgBegin(dkg_begin) = &packet.msg { + if self.current_dkg_id >= dkg_begin.dkg_id { + // We have already processed this DKG round + return Ok((None, None)); + } // Set the current sign id to one before the current message to ensure // that we start the next round at the correct id. (Do this rather // then overwriting afterwards to ensure logging is accurate) @@ -68,6 +72,10 @@ impl Coordinator { let packet = self.start_dkg_round()?; return Ok((Some(packet), None)); } else if let Message::NonceRequest(nonce_request) = &packet.msg { + if self.current_sign_id >= nonce_request.sign_id { + // We have already processed this sign round + return Ok((None, None)); + } // Set the current sign id to one before the current message to ensure // that we start the next round at the correct id. (Do this rather // then overwriting afterwards to ensure logging is accurate) @@ -747,7 +755,7 @@ impl CoordinatorTrait for Coordinator { pub mod test { use crate::{ curve::scalar::Scalar, - net::Message, + net::{DkgBegin, Message, NonceRequest, Packet}, state_machine::coordinator::{ frost::Coordinator as FrostCoordinator, test::{ @@ -857,4 +865,85 @@ pub mod test { fn process_inbound_messages_v2() { run_dkg_sign::, v2::Signer>(5, 2); } + + #[test] + fn old_round_ids_are_ignored_v1() { + old_round_ids_are_ignored::(); + } + + #[test] + fn old_round_ids_are_ignored_v2() { + old_round_ids_are_ignored::(); + } + + fn old_round_ids_are_ignored() { + let mut rng = OsRng; + let config = Config::new(10, 40, 28, Scalar::random(&mut rng)); + let mut coordinator = FrostCoordinator::::new(config); + let id: u64 = 10; + let old_id = id.saturating_sub(1); + coordinator.current_dkg_id = id; + coordinator.current_sign_id = id; + // Attempt to start an old DKG round + let (packets, results) = coordinator + .process_inbound_messages(&[Packet { + sig: vec![], + msg: Message::DkgBegin(DkgBegin { dkg_id: old_id }), + }]) + .unwrap(); + assert!(packets.is_empty()); + assert!(results.is_empty()); + assert_eq!(coordinator.state, State::Idle); + assert_eq!(coordinator.current_dkg_id, id); + + // Attempt to start the same DKG round + let (packets, results) = coordinator + .process_inbound_messages(&[Packet { + sig: vec![], + msg: Message::DkgBegin(DkgBegin { dkg_id: id }), + }]) + .unwrap(); + assert!(packets.is_empty()); + assert!(results.is_empty()); + assert_eq!(coordinator.state, State::Idle); + assert_eq!(coordinator.current_dkg_id, id); + + // Attempt to start an old Sign round + let (packets, results) = coordinator + .process_inbound_messages(&[Packet { + sig: vec![], + msg: Message::NonceRequest(NonceRequest { + dkg_id: id, + sign_id: old_id, + message: vec![], + sign_iter_id: id, + is_taproot: false, + merkle_root: None, + }), + }]) + .unwrap(); + assert!(packets.is_empty()); + assert!(results.is_empty()); + assert_eq!(coordinator.state, State::Idle); + assert_eq!(coordinator.current_sign_id, id); + + // Attempt to start the same Sign round + let (packets, results) = coordinator + .process_inbound_messages(&[Packet { + sig: vec![], + msg: Message::NonceRequest(NonceRequest { + dkg_id: id, + sign_id: id, + message: vec![], + sign_iter_id: id, + is_taproot: false, + merkle_root: None, + }), + }]) + .unwrap(); + assert!(packets.is_empty()); + assert!(results.is_empty()); + assert_eq!(coordinator.state, State::Idle); + assert_eq!(coordinator.current_sign_id, id); + } }