From acaafd19031c686a3bde83ae454c25fad6158ef3 Mon Sep 17 00:00:00 2001 From: David Steiner Date: Mon, 8 Dec 2025 12:54:51 +0100 Subject: [PATCH 1/7] Fix handling of sequence resets when they are not gap fills --- crates/hotfix/src/message/verification.rs | 54 ++++++++++++----------- crates/hotfix/src/session.rs | 43 +++++++++++++----- 2 files changed, 60 insertions(+), 37 deletions(-) diff --git a/crates/hotfix/src/message/verification.rs b/crates/hotfix/src/message/verification.rs index d2b0ce1..63eb591 100644 --- a/crates/hotfix/src/message/verification.rs +++ b/crates/hotfix/src/message/verification.rs @@ -13,7 +13,7 @@ const SENDING_TIME_THRESHOLD: u64 = 120; pub(crate) fn verify_message( message: &Message, config: &SessionConfig, - expected_seq_number: u64, + expected_seq_number: Option, ) -> Result<(), MessageVerificationError> { check_begin_string(message, config.begin_string.as_str())?; let actual_seq_number: u64 = message.header().get(fix44::MSG_SEQ_NUM).unwrap_or_default(); @@ -33,7 +33,9 @@ pub(crate) fn verify_message( check_original_sending_time(message, actual_seq_number, sending_time)?; } - check_sequence_number(actual_seq_number, expected_seq_number, possible_duplicate)?; + if let Some(expected_seq_number) = expected_seq_number { + check_sequence_number(actual_seq_number, expected_seq_number, possible_duplicate)?; + } Ok(()) } @@ -217,7 +219,7 @@ mod tests { let config = build_test_config(); let msg = build_test_message("FIX.4.4", "TARGET", "SENDER", 42); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(result.is_ok()); } @@ -227,7 +229,7 @@ mod tests { let config = build_test_config(); let msg = build_test_message("FIX.4.2", "TARGET", "SENDER", 42); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -243,7 +245,7 @@ mod tests { let config = build_test_config(); let msg = build_test_message("FIX.4.4", "WRONG_SENDER", "SENDER", 42); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -269,7 +271,7 @@ mod tests { let config = build_test_config(); let msg = build_test_message("FIX.4.4", "TARGET", "WRONG_TARGET", 42); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -295,7 +297,7 @@ mod tests { let config = build_test_config(); let msg = build_test_message("FIX.4.4", "TARGET", "SENDER", 40); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -321,7 +323,7 @@ mod tests { msg.header_mut() .set(fix44::ORIG_SENDING_TIME, Timestamp::utc_now()); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -344,7 +346,7 @@ mod tests { let config = build_test_config(); let msg = build_test_message("FIX.4.4", "TARGET", "SENDER", 50); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -363,7 +365,7 @@ mod tests { msg.header_mut().set(fix44::POSS_DUP_FLAG, true); // Don't set OrigSendingTime - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -388,7 +390,7 @@ mod tests { msg.header_mut().pop(fix44::SENDING_TIME); msg.header_mut().set(fix44::SENDING_TIME, sending_time); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(result.is_ok()); } @@ -407,7 +409,7 @@ mod tests { msg.header_mut().pop(fix44::SENDING_TIME); msg.header_mut().set(fix44::SENDING_TIME, sending_time); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -437,7 +439,7 @@ mod tests { msg.header_mut().pop(fix44::SENDING_TIME); msg.header_mut().set(fix44::SENDING_TIME, timestamp); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); // equal timestamps should be valid (orig <= sending) assert!(result.is_ok()); @@ -455,7 +457,7 @@ mod tests { // remove begin string, which is automatically added by `Message::new` msg.header_mut().pop(fix44::BEGIN_STRING); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -471,7 +473,7 @@ mod tests { msg.set(fix44::MSG_SEQ_NUM, 42u64); msg.set(fix44::SENDING_TIME, Timestamp::utc_now()); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -490,7 +492,7 @@ mod tests { msg.set(fix44::MSG_SEQ_NUM, 42u64); msg.set(fix44::SENDING_TIME, Timestamp::utc_now()); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -509,7 +511,7 @@ mod tests { msg.set(fix44::TARGET_COMP_ID, "SENDER"); msg.set(fix44::SENDING_TIME, Timestamp::utc_now()); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); // missing seq num defaults to 0, which will be too low assert!(matches!( @@ -523,7 +525,7 @@ mod tests { let config = build_test_config(); let msg = build_test_message("FIX.4.4", "TARGET", "SENDER", 0); - let result = verify_message(&msg, &config, 1); + let result = verify_message(&msg, &config, Some(1)); assert!(matches!( result, @@ -536,7 +538,7 @@ mod tests { let config = build_test_config(); let msg = build_test_message("FIX.4.4", "TARGET", "SENDER", 1); - let result = verify_message(&msg, &config, 1); + let result = verify_message(&msg, &config, Some(1)); assert!(result.is_ok()); } @@ -547,7 +549,7 @@ mod tests { // wrong begin string AND wrong seq num - begin string error should come first let msg = build_test_message("FIX.4.2", "TARGET", "SENDER", 100); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -561,7 +563,7 @@ mod tests { // wrong sender and wrong target - sender error should come first let msg = build_test_message("FIX.4.4", "WRONG_SENDER", "WRONG_TARGET", 42); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -580,7 +582,7 @@ mod tests { msg.set(fix44::TARGET_COMP_ID, "SENDER"); msg.set(fix44::MSG_SEQ_NUM, 42u64); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -607,7 +609,7 @@ mod tests { let past_timestamp: Timestamp = past_time.naive_utc().into(); msg.set(fix44::SENDING_TIME, past_timestamp); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -634,7 +636,7 @@ mod tests { let future_timestamp: Timestamp = future_time.naive_utc().into(); msg.set(fix44::SENDING_TIME, future_timestamp); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(matches!( result, @@ -661,7 +663,7 @@ mod tests { let boundary_timestamp: Timestamp = boundary_time.naive_utc().into(); msg.set(fix44::SENDING_TIME, boundary_timestamp); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(result.is_ok()); } @@ -682,7 +684,7 @@ mod tests { let valid_timestamp: Timestamp = valid_time.naive_utc().into(); msg.set(fix44::SENDING_TIME, valid_timestamp); - let result = verify_message(&msg, &config, 42); + let result = verify_message(&msg, &config, Some(42)); assert!(result.is_ok()); } diff --git a/crates/hotfix/src/session.rs b/crates/hotfix/src/session.rs index 8d98835..710285b 100644 --- a/crates/hotfix/src/session.rs +++ b/crates/hotfix/src/session.rs @@ -167,8 +167,6 @@ impl, M: FixMessage, S: MessageStore> Session { let message_type = message.header().get(fix44::MSG_TYPE)?; if let SessionState::AwaitingResend(state) = &mut self.state { - // TODO: consider what messages won't have a sequence number? - // e.g. SequenceReset? let seq_number: u64 = message .header() .get(fix44::MSG_SEQ_NUM) @@ -216,7 +214,7 @@ impl, M: FixMessage, S: MessageStore> Session { } async fn process_app_message(&mut self, message: &Message) -> Result<()> { - match self.verify_message(message) { + match self.verify_message(message, true) { Ok(_) => { let parsed_message = M::parse(message); if matches!( @@ -268,8 +266,14 @@ impl, M: FixMessage, S: MessageStore> Session { fn verify_message( &self, message: &Message, + verify_target_seq_number: bool, ) -> std::result::Result<(), MessageVerificationError> { - verify_message(message, &self.config, self.store.next_target_seq_number()) + let expected_seq_number = if verify_target_seq_number { + Some(self.store.next_target_seq_number()) + } else { + None + }; + verify_message(message, &self.config, expected_seq_number) } async fn on_connect(&mut self, writer: WriterRef) { @@ -300,9 +304,8 @@ impl, M: FixMessage, S: MessageStore> Session { } async fn on_logon(&mut self, message: &Message) -> Result<()> { - // TODO: this should wait to see if a resend request is sent if let SessionState::AwaitingLogon { writer, .. } = &self.state { - match self.verify_message(message) { + match self.verify_message(message, true) { Ok(_) => { // happy logon flow, the session is now active self.state = @@ -433,13 +436,31 @@ impl, M: FixMessage, S: MessageStore> Session { } async fn on_sequence_reset(&mut self, message: &Message) -> Result<()> { - let gap_fill: bool = message.get(fix44::GAP_FILL_FLAG).unwrap(); - if !gap_fill { - // TODO: non gap fill is valid as well of course, but I don't yet know the use-case for it is - panic!("expected sequence reset with gap fill"); + let is_gap_fill: bool = message.get(fix44::GAP_FILL_FLAG).unwrap(); + if let Err(err) = self.verify_message(message, is_gap_fill) { + self.handle_verification_error(err).await; + return Ok(()); } - let end: u64 = message.get(fix44::NEW_SEQ_NO).unwrap(); + let end: u64 = match message.get(fix44::NEW_SEQ_NO) { + Ok(new_seq_no) => new_seq_no, + Err(err) => { + error!( + "received sequence reset message without new sequence number: {:?}", + err + ); + let reject = Reject::new( + message + .header() + .get(fix44::MSG_SEQ_NUM) + .map_err(|_| anyhow!("failed to get seq number"))?, + ) + .session_reject_reason(SessionRejectReason::RequiredTagMissing) + .text("missing NewSeqNo tag in sequence reset message"); + self.send_message(reject).await; + return Ok(()); + } + }; self.store.set_target_seq_number(end - 1).await } From 057db1e4023b1795d30a518410309b98ce47574a Mon Sep 17 00:00:00 2001 From: David Steiner Date: Mon, 8 Dec 2025 13:06:46 +0100 Subject: [PATCH 2/7] Add test case for happy gap fill flow during resend --- crates/hotfix/tests/session_test_cases/mod.rs | 1 + .../sequence_reset_tests.rs | 67 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 crates/hotfix/tests/session_test_cases/sequence_reset_tests.rs diff --git a/crates/hotfix/tests/session_test_cases/mod.rs b/crates/hotfix/tests/session_test_cases/mod.rs index 7162044..badcda1 100644 --- a/crates/hotfix/tests/session_test_cases/mod.rs +++ b/crates/hotfix/tests/session_test_cases/mod.rs @@ -5,3 +5,4 @@ mod invalid_message_tests; mod logon_tests; mod logout_tests; mod resend_tests; +mod sequence_reset_tests; diff --git a/crates/hotfix/tests/session_test_cases/sequence_reset_tests.rs b/crates/hotfix/tests/session_test_cases/sequence_reset_tests.rs new file mode 100644 index 0000000..95262bb --- /dev/null +++ b/crates/hotfix/tests/session_test_cases/sequence_reset_tests.rs @@ -0,0 +1,67 @@ +use crate::common::actions::when; +use crate::common::assertions::{assert_msg_type, then}; +use crate::common::cleanup::finally; +use crate::common::setup::given_an_active_session; +use crate::common::test_messages::TestMessage; +use hotfix::session::Status; +use hotfix_message::fix44::MsgType; + +/// Tests that the session correctly processes an inbound SequenceReset-GapFill message. +/// +/// This test verifies the workflow where: +/// 1. An active session is established +/// 2. The counterparty previously sent admin messages (e.g., heartbeats) that we missed +/// 3. The counterparty sends a business message with a higher sequence number +/// 4. We detect the gap and send a ResendRequest +/// 5. The counterparty responds with a SequenceReset-GapFill to skip the admin messages +/// 6. We process the gap fill and update our target sequence number correctly +/// 7. The session transitions back to Active status +#[tokio::test] +async fn test_correct_inbound_sequence_reset_with_gap_fill() { + let (mut session, mut counterparty) = given_an_active_session().await; + + // the counterparty previously sent admin messages (e.g., heartbeats) which we missed + // we'll simulate this by having them skip sequence numbers 2 and 3 + when(&mut counterparty) + .has_previously_sent(TestMessage::dummy_execution_report()) + .await; + when(&mut counterparty) + .has_previously_sent(TestMessage::dummy_execution_report()) + .await; + + // the counterparty now sends a business message with sequence number 4 + when(&mut counterparty) + .sends_message(TestMessage::dummy_execution_report()) + .await; + + // we detect the gap and transition to AwaitingResend state + then(&mut session) + .status_changes_to(Status::AwaitingResend { + begin: 2, + end: 4, + attempts: 1, + }) + .await; + + // we send a ResendRequest to the counterparty + then(&mut counterparty) + .receives(|msg| assert_msg_type(msg, MsgType::ResendRequest)) + .await; + + // the counterparty responds with a SequenceReset-GapFill for messages 2-3 + // indicating that messages 2 and 3 were admin messages that don't need to be resent + // NewSeqNo=4 means the next message after the gap is sequence number 4 + when(&mut counterparty).sends_gap_fill(2, 4).await; + + // the counterparty also needs to resend message 4 (the business message) + when(&mut counterparty).resends_message(4).await; + + // the session should process the gap fill and the resent message, then transition back to Active + then(&mut session).status_changes_to(Status::Active).await; + + // verify that our target sequence number has been updated correctly + // we should now expect sequence number 5 (after receiving 1=logon, 2-3=gap filled, 4=resent) + then(&mut session).target_sequence_number_reaches(4).await; + + finally(&session, &mut counterparty).disconnect().await; +} From 0fe25ba031a59ce81417bf8782701685d1772b9e Mon Sep 17 00:00:00 2001 From: David Steiner Date: Mon, 8 Dec 2025 13:36:25 +0100 Subject: [PATCH 3/7] Better test cases for sequence number mismatches during sequence resets --- crates/hotfix/tests/session_test_cases/mod.rs | 3 +- .../received_gap_fill_tests.rs | 182 ++++++++++++++++++ .../received_reset_tests.rs | 4 + .../sequence_reset_tests.rs | 67 ------- 4 files changed, 188 insertions(+), 68 deletions(-) create mode 100644 crates/hotfix/tests/session_test_cases/received_gap_fill_tests.rs create mode 100644 crates/hotfix/tests/session_test_cases/received_reset_tests.rs delete mode 100644 crates/hotfix/tests/session_test_cases/sequence_reset_tests.rs diff --git a/crates/hotfix/tests/session_test_cases/mod.rs b/crates/hotfix/tests/session_test_cases/mod.rs index badcda1..4fbc7c3 100644 --- a/crates/hotfix/tests/session_test_cases/mod.rs +++ b/crates/hotfix/tests/session_test_cases/mod.rs @@ -4,5 +4,6 @@ mod heartbeat_tests; mod invalid_message_tests; mod logon_tests; mod logout_tests; +mod received_gap_fill_tests; +mod received_reset_tests; mod resend_tests; -mod sequence_reset_tests; diff --git a/crates/hotfix/tests/session_test_cases/received_gap_fill_tests.rs b/crates/hotfix/tests/session_test_cases/received_gap_fill_tests.rs new file mode 100644 index 0000000..278f7b9 --- /dev/null +++ b/crates/hotfix/tests/session_test_cases/received_gap_fill_tests.rs @@ -0,0 +1,182 @@ +//! Tests for handling inbound gap fill messages. +//! +//! These tests are only concerned with gap fills, +//! that is `SequenceReset` messages with the `GapFillFlag` set to `Y`. +use crate::common::actions::when; +use crate::common::assertions::{assert_msg_type, then}; +use crate::common::cleanup::finally; +use crate::common::setup::given_an_active_session; +use crate::common::test_messages::TestMessage; +use hotfix::session::Status; +use hotfix_message::fix44::MsgType; + +/// Tests that the session correctly processes an inbound SequenceReset-GapFill message. +#[tokio::test] +async fn test_correct_inbound_sequence_reset_with_gap_fill() { + let (mut session, mut counterparty) = given_an_active_session().await; + + // the counterparty previously sent admin messages (e.g., heartbeats) which we missed + // we'll simulate this by having them skip sequence numbers 2 and 3 + when(&mut counterparty) + .has_previously_sent(TestMessage::dummy_execution_report()) + .await; + when(&mut counterparty) + .has_previously_sent(TestMessage::dummy_execution_report()) + .await; + + // the counterparty now sends a business message with sequence number 4 + when(&mut counterparty) + .sends_message(TestMessage::dummy_execution_report()) + .await; + + // we detect the gap and transition to AwaitingResend state + then(&mut session) + .status_changes_to(Status::AwaitingResend { + begin: 2, + end: 4, + attempts: 1, + }) + .await; + + // we send a ResendRequest to the counterparty + then(&mut counterparty) + .receives(|msg| assert_msg_type(msg, MsgType::ResendRequest)) + .await; + + // the counterparty responds with a SequenceReset-GapFill for messages 2-3 + // indicating that messages 2 and 3 were admin messages that don't need to be resent + // NewSeqNo=4 means the next message after the gap is sequence number 4 + when(&mut counterparty).sends_gap_fill(2, 4).await; + + // the counterparty also needs to resend message 4 (the business message) + when(&mut counterparty).resends_message(4).await; + + // the session should process the gap fill and the resent message, then transition back to Active + then(&mut session).status_changes_to(Status::Active).await; + + // verify that our target sequence number has been updated correctly + // we should now expect sequence number 5 (after receiving 1=logon, 2-3=gap filled, 4=resent) + then(&mut session).target_sequence_number_reaches(4).await; + + finally(&session, &mut counterparty).disconnect().await; +} + +/// Tests that the session issues a new resend request when the incoming sequence reset's +/// sequence number is too high. +#[tokio::test] +async fn test_sequence_reset_with_sequence_number_too_high_during_resend() { + let (mut session, mut counterparty) = given_an_active_session().await; + + // the counterparty previously sent admin messages (e.g., heartbeats) which we missed + // we'll simulate this by having them skip sequence numbers 2 and 3 + when(&mut counterparty) + .has_previously_sent(TestMessage::dummy_execution_report()) + .await; + when(&mut counterparty) + .has_previously_sent(TestMessage::dummy_execution_report()) + .await; + + // the counterparty now sends a business message with sequence number 4 + when(&mut counterparty) + .sends_message(TestMessage::dummy_execution_report()) + .await; + + // we detect the gap and transition to AwaitingResend state + then(&mut session) + .status_changes_to(Status::AwaitingResend { + begin: 2, + end: 4, + attempts: 1, + }) + .await; + + // we send a ResendRequest to the counterparty for messages 2-4 + then(&mut counterparty) + .receives(|msg| assert_msg_type(msg, MsgType::ResendRequest)) + .await; + + // the counterparty responds with a SequenceReset-GapFill, but with an INCORRECT sequence number + // instead of starting at sequence 2 (the beginning of the gap), it incorrectly starts at 3 + when(&mut counterparty).sends_gap_fill(3, 5).await; + + // the session rejects this invalid gap fill by detecting the sequence number mismatch + // and requesting another resend for the still-missing message 2 + then(&mut counterparty) + .receives(|msg| assert_msg_type(msg, MsgType::ResendRequest)) + .await; + + // verify the session is still in AwaitingResend state, now with updated parameters + // and incremented attempts (from 1 to 2) + then(&mut session) + .status_changes_to(Status::AwaitingResend { + begin: 2, + end: 3, + attempts: 2, + }) + .await; + + // now send the correct gap fill and resend to complete the recovery + when(&mut counterparty).sends_gap_fill(2, 3).await; + when(&mut counterparty).resends_message(3).await; + then(&mut session).status_changes_to(Status::Active).await; + + finally(&session, &mut counterparty).disconnect().await; +} + +/// Tests that the session ignores a SequenceReset-GapFill with a sequence number too low. +/// +/// This is only true if the sequence reset is otherwise valid with the `PossDupFlag` +/// set to `Y`. +#[tokio::test] +async fn test_reject_sequence_reset_with_sequence_number_too_low_is_ignored_during_resend() { + let (mut session, mut counterparty) = given_an_active_session().await; + + // the counterparty previously sent admin messages (e.g., heartbeats) which we missed + // we'll simulate this by having them skip sequence numbers 2 and 3 + when(&mut counterparty) + .has_previously_sent(TestMessage::dummy_execution_report()) + .await; + when(&mut counterparty) + .has_previously_sent(TestMessage::dummy_execution_report()) + .await; + + // the counterparty now sends a business message with sequence number 4 + when(&mut counterparty) + .sends_message(TestMessage::dummy_execution_report()) + .await; + + // we detect the gap and transition to AwaitingResend state + then(&mut session) + .status_changes_to(Status::AwaitingResend { + begin: 2, + end: 4, + attempts: 1, + }) + .await; + + // we send a ResendRequest to the counterparty for messages 2-4 + then(&mut counterparty) + .receives(|msg| assert_msg_type(msg, MsgType::ResendRequest)) + .await; + + // the counterparty responds with a SequenceReset-GapFill, but with an incorrect sequence number + // instead of starting at sequence 2 (the beginning of the gap), it incorrectly starts at 1 + when(&mut counterparty).sends_gap_fill(1, 4).await; + + // the session ignores this invalid gap fill (logged as PossDup with seq too low) + // we verify this by checking the state hasn't changed + then(&mut session) + .status_changes_to(Status::AwaitingResend { + begin: 2, + end: 4, + attempts: 1, + }) + .await; + + // now send the correct gap fill and resend to complete the recovery + when(&mut counterparty).sends_gap_fill(2, 4).await; + when(&mut counterparty).resends_message(4).await; + then(&mut session).status_changes_to(Status::Active).await; + + finally(&session, &mut counterparty).disconnect().await; +} diff --git a/crates/hotfix/tests/session_test_cases/received_reset_tests.rs b/crates/hotfix/tests/session_test_cases/received_reset_tests.rs new file mode 100644 index 0000000..d8f9dc9 --- /dev/null +++ b/crates/hotfix/tests/session_test_cases/received_reset_tests.rs @@ -0,0 +1,4 @@ +//! Tests for handling inbound Reset messages. +//! +//! These tests are only concerned with true resets, +//! that is `SequenceReset` messages without the `GapFillFlag` set. diff --git a/crates/hotfix/tests/session_test_cases/sequence_reset_tests.rs b/crates/hotfix/tests/session_test_cases/sequence_reset_tests.rs deleted file mode 100644 index 95262bb..0000000 --- a/crates/hotfix/tests/session_test_cases/sequence_reset_tests.rs +++ /dev/null @@ -1,67 +0,0 @@ -use crate::common::actions::when; -use crate::common::assertions::{assert_msg_type, then}; -use crate::common::cleanup::finally; -use crate::common::setup::given_an_active_session; -use crate::common::test_messages::TestMessage; -use hotfix::session::Status; -use hotfix_message::fix44::MsgType; - -/// Tests that the session correctly processes an inbound SequenceReset-GapFill message. -/// -/// This test verifies the workflow where: -/// 1. An active session is established -/// 2. The counterparty previously sent admin messages (e.g., heartbeats) that we missed -/// 3. The counterparty sends a business message with a higher sequence number -/// 4. We detect the gap and send a ResendRequest -/// 5. The counterparty responds with a SequenceReset-GapFill to skip the admin messages -/// 6. We process the gap fill and update our target sequence number correctly -/// 7. The session transitions back to Active status -#[tokio::test] -async fn test_correct_inbound_sequence_reset_with_gap_fill() { - let (mut session, mut counterparty) = given_an_active_session().await; - - // the counterparty previously sent admin messages (e.g., heartbeats) which we missed - // we'll simulate this by having them skip sequence numbers 2 and 3 - when(&mut counterparty) - .has_previously_sent(TestMessage::dummy_execution_report()) - .await; - when(&mut counterparty) - .has_previously_sent(TestMessage::dummy_execution_report()) - .await; - - // the counterparty now sends a business message with sequence number 4 - when(&mut counterparty) - .sends_message(TestMessage::dummy_execution_report()) - .await; - - // we detect the gap and transition to AwaitingResend state - then(&mut session) - .status_changes_to(Status::AwaitingResend { - begin: 2, - end: 4, - attempts: 1, - }) - .await; - - // we send a ResendRequest to the counterparty - then(&mut counterparty) - .receives(|msg| assert_msg_type(msg, MsgType::ResendRequest)) - .await; - - // the counterparty responds with a SequenceReset-GapFill for messages 2-3 - // indicating that messages 2 and 3 were admin messages that don't need to be resent - // NewSeqNo=4 means the next message after the gap is sequence number 4 - when(&mut counterparty).sends_gap_fill(2, 4).await; - - // the counterparty also needs to resend message 4 (the business message) - when(&mut counterparty).resends_message(4).await; - - // the session should process the gap fill and the resent message, then transition back to Active - then(&mut session).status_changes_to(Status::Active).await; - - // verify that our target sequence number has been updated correctly - // we should now expect sequence number 5 (after receiving 1=logon, 2-3=gap filled, 4=resent) - then(&mut session).target_sequence_number_reaches(4).await; - - finally(&session, &mut counterparty).disconnect().await; -} From c5eea2eca070ba672c16612eeca52ced44bf2cf0 Mon Sep 17 00:00:00 2001 From: David Steiner Date: Mon, 8 Dec 2025 13:53:48 +0100 Subject: [PATCH 4/7] Add test case for happy reset flow --- crates/hotfix/tests/common/actions.rs | 10 ++++++- .../tests/common/fakes/fake_counterparty.rs | 9 ++++-- .../received_gap_fill_tests.rs | 3 ++ .../received_reset_tests.rs | 28 +++++++++++++++++++ 4 files changed, 47 insertions(+), 3 deletions(-) diff --git a/crates/hotfix/tests/common/actions.rs b/crates/hotfix/tests/common/actions.rs index 09ffb0e..1c3cf73 100644 --- a/crates/hotfix/tests/common/actions.rs +++ b/crates/hotfix/tests/common/actions.rs @@ -47,7 +47,15 @@ impl When<&mut FakeCounterparty> { } pub async fn sends_gap_fill(&mut self, start_seq_no: u64, new_seq_no: u64) { - self.target.send_gap_fill(start_seq_no, new_seq_no).await; + self.target + .send_sequence_reset(start_seq_no, new_seq_no, true) + .await; + } + + pub async fn sends_sequence_reset(&mut self, start_seq_no: u64, new_seq_no: u64) { + self.target + .send_sequence_reset(start_seq_no, new_seq_no, false) + .await; } pub async fn sends_logon(&mut self) { diff --git a/crates/hotfix/tests/common/fakes/fake_counterparty.rs b/crates/hotfix/tests/common/fakes/fake_counterparty.rs index 02ff1d0..4621592 100644 --- a/crates/hotfix/tests/common/fakes/fake_counterparty.rs +++ b/crates/hotfix/tests/common/fakes/fake_counterparty.rs @@ -117,9 +117,14 @@ where } } - pub async fn send_gap_fill(&mut self, start_seq_no: u64, new_seq_no: u64) { + pub async fn send_sequence_reset( + &mut self, + start_seq_no: u64, + new_seq_no: u64, + gap_fill: bool, + ) { let sequence_reset = SequenceReset { - gap_fill: true, + gap_fill, new_seq_no, }; let raw_message = generate_message( diff --git a/crates/hotfix/tests/session_test_cases/received_gap_fill_tests.rs b/crates/hotfix/tests/session_test_cases/received_gap_fill_tests.rs index 278f7b9..08ec74f 100644 --- a/crates/hotfix/tests/session_test_cases/received_gap_fill_tests.rs +++ b/crates/hotfix/tests/session_test_cases/received_gap_fill_tests.rs @@ -2,6 +2,9 @@ //! //! These tests are only concerned with gap fills, //! that is `SequenceReset` messages with the `GapFillFlag` set to `Y`. +//! +//! These correspond to the test cases in +//! [Scenario 10](https://www.fixtrading.org/standards/fix-session-testcases-online/#scenario-10-receive-sequence-reset-gap-fill). use crate::common::actions::when; use crate::common::assertions::{assert_msg_type, then}; use crate::common::cleanup::finally; diff --git a/crates/hotfix/tests/session_test_cases/received_reset_tests.rs b/crates/hotfix/tests/session_test_cases/received_reset_tests.rs index d8f9dc9..ae821ed 100644 --- a/crates/hotfix/tests/session_test_cases/received_reset_tests.rs +++ b/crates/hotfix/tests/session_test_cases/received_reset_tests.rs @@ -2,3 +2,31 @@ //! //! These tests are only concerned with true resets, //! that is `SequenceReset` messages without the `GapFillFlag` set. +//! +//! These correspond to the test cases in +//! [Scenario 11](https://www.fixtrading.org/standards/fix-session-testcases-online/#scenario-11-receive-sequence-reset-reset). +use tokio::test; + +use crate::common::actions::when; +use crate::common::assertions::then; +use crate::common::cleanup::finally; +use crate::common::setup::given_an_active_session; + +/// Tests that the session correctly processes an inbound SequenceReset message +/// with `NewSeqNo` higher than the current target sequence number. +/// +/// It should set the target sequence number to the new value. +#[test] +async fn test_receive_reset_with_new_seq_number_higher_than_current() { + const NEW_SEQ_NO: u64 = 10; + let (mut session, mut counterparty) = given_an_active_session().await; + + when(&mut counterparty) + .sends_sequence_reset(1, NEW_SEQ_NO) + .await; + then(&mut session) + .target_sequence_number_reaches(NEW_SEQ_NO - 1) + .await; + + finally(&session, &mut counterparty).disconnect().await; +} From d55199fd8f9c004aaedcbe8368bf02dfc06d206e Mon Sep 17 00:00:00 2001 From: David Steiner Date: Mon, 8 Dec 2025 14:12:34 +0100 Subject: [PATCH 5/7] Add test case for processing sequence reset with sequence number lower than expected --- .../received_reset_tests.rs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/crates/hotfix/tests/session_test_cases/received_reset_tests.rs b/crates/hotfix/tests/session_test_cases/received_reset_tests.rs index ae821ed..1412685 100644 --- a/crates/hotfix/tests/session_test_cases/received_reset_tests.rs +++ b/crates/hotfix/tests/session_test_cases/received_reset_tests.rs @@ -11,6 +11,7 @@ use crate::common::actions::when; use crate::common::assertions::then; use crate::common::cleanup::finally; use crate::common::setup::given_an_active_session; +use crate::common::test_messages::TestMessage; /// Tests that the session correctly processes an inbound SequenceReset message /// with `NewSeqNo` higher than the current target sequence number. @@ -30,3 +31,35 @@ async fn test_receive_reset_with_new_seq_number_higher_than_current() { finally(&session, &mut counterparty).disconnect().await; } + +/// Tests that the reset is processed even when the sequence number is off. +/// +/// For example, a sequence number too low would normally result in a termination +/// of the session, but for non-gap fill resets, it is happily accepted. +#[test] +async fn test_sequence_number_is_ignored_in_resets() { + let (mut session, mut counterparty) = given_an_active_session().await; + + // the counterparty sends a message, but due to a failure, it's lost + let sequence_number = counterparty.next_target_sequence_number(); + when(&mut counterparty) + .sends_message(TestMessage::dummy_execution_report()) + .await; + then(&mut session) + .target_sequence_number_reaches(sequence_number) + .await; + counterparty.delete_last_message_from_store(); + + // the counterparty sends a reset with the sequence number being the same + // as the previous message's + // the new sequence number is higher than what we currently have + let new_sequence_number = sequence_number + 10; + when(&mut counterparty) + .sends_sequence_reset(sequence_number, new_sequence_number) + .await; + then(&mut session) + .target_sequence_number_reaches(new_sequence_number - 1) + .await; + + finally(&session, &mut counterparty).disconnect().await; +} From 1a7e898b20367fcd5a6af9f5236800ada8a3a3ad Mon Sep 17 00:00:00 2001 From: David Steiner Date: Mon, 8 Dec 2025 14:29:13 +0100 Subject: [PATCH 6/7] Add test case for new seq number being lower than expected in resets --- crates/hotfix/src/session.rs | 33 +++++++++++---- .../received_reset_tests.rs | 42 +++++++++++++++++-- 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/crates/hotfix/src/session.rs b/crates/hotfix/src/session.rs index 710285b..4026aa2 100644 --- a/crates/hotfix/src/session.rs +++ b/crates/hotfix/src/session.rs @@ -436,7 +436,11 @@ impl, M: FixMessage, S: MessageStore> Session { } async fn on_sequence_reset(&mut self, message: &Message) -> Result<()> { - let is_gap_fill: bool = message.get(fix44::GAP_FILL_FLAG).unwrap(); + let msg_seq_num = message + .header() + .get(fix44::MSG_SEQ_NUM) + .map_err(|_| anyhow!("failed to get seq number"))?; + let is_gap_fill: bool = message.get(fix44::GAP_FILL_FLAG).unwrap_or(false); if let Err(err) = self.verify_message(message, is_gap_fill) { self.handle_verification_error(err).await; return Ok(()); @@ -449,18 +453,29 @@ impl, M: FixMessage, S: MessageStore> Session { "received sequence reset message without new sequence number: {:?}", err ); - let reject = Reject::new( - message - .header() - .get(fix44::MSG_SEQ_NUM) - .map_err(|_| anyhow!("failed to get seq number"))?, - ) - .session_reject_reason(SessionRejectReason::RequiredTagMissing) - .text("missing NewSeqNo tag in sequence reset message"); + let reject = Reject::new(msg_seq_num) + .session_reject_reason(SessionRejectReason::RequiredTagMissing) + .text("missing NewSeqNo tag in sequence reset message"); self.send_message(reject).await; return Ok(()); } }; + + // sequence resets cannot move the target seq number backwards + // regardless of whether the message is a gap fill or not + if end <= self.store.next_target_seq_number() { + error!( + "received sequence reset message which would move target seq number backwards: {end}", + ); + let text = + format!("attempt to lower sequence number, invalid value NewSeqNo(36)={end}"); + let reject = Reject::new(msg_seq_num) + .session_reject_reason(SessionRejectReason::ValueIsIncorrect) + .text(&text); + self.send_message(reject).await; + return Ok(()); + } + self.store.set_target_seq_number(end - 1).await } diff --git a/crates/hotfix/tests/session_test_cases/received_reset_tests.rs b/crates/hotfix/tests/session_test_cases/received_reset_tests.rs index 1412685..cb66127 100644 --- a/crates/hotfix/tests/session_test_cases/received_reset_tests.rs +++ b/crates/hotfix/tests/session_test_cases/received_reset_tests.rs @@ -5,13 +5,13 @@ //! //! These correspond to the test cases in //! [Scenario 11](https://www.fixtrading.org/standards/fix-session-testcases-online/#scenario-11-receive-sequence-reset-reset). -use tokio::test; - use crate::common::actions::when; -use crate::common::assertions::then; +use crate::common::assertions::{assert_msg_type, then}; use crate::common::cleanup::finally; use crate::common::setup::given_an_active_session; use crate::common::test_messages::TestMessage; +use hotfix_message::fix44::MsgType; +use tokio::test; /// Tests that the session correctly processes an inbound SequenceReset message /// with `NewSeqNo` higher than the current target sequence number. @@ -63,3 +63,39 @@ async fn test_sequence_number_is_ignored_in_resets() { finally(&session, &mut counterparty).disconnect().await; } + +#[test] +async fn test_reset_moving_sequence_number_back_is_rejected() { + let (mut session, mut counterparty) = given_an_active_session().await; + + // the counterparty sends a valid message + let sequence_number = counterparty.next_target_sequence_number(); + when(&mut counterparty) + .sends_message(TestMessage::dummy_execution_report()) + .await; + then(&mut session) + .target_sequence_number_reaches(sequence_number) + .await; + + // the counterparty then tries to reset our sequence number back to a value + // lower than what we think it should be + when(&mut counterparty) + .sends_sequence_reset(sequence_number + 1, 1) + .await; + + // which gets rejected + then(&mut counterparty) + .receives(|msg| assert_msg_type(msg, MsgType::Reject)) + .await; + + // but the session remains active, and we're able to process subsequent messages + let sequence_number = counterparty.next_target_sequence_number(); + when(&mut counterparty) + .sends_message(TestMessage::dummy_execution_report()) + .await; + then(&mut session) + .target_sequence_number_reaches(sequence_number) + .await; + + finally(&session, &mut counterparty).disconnect().await; +} From 2038236a4fda648a8340d07647e8b45386639e1e Mon Sep 17 00:00:00 2001 From: David Steiner Date: Mon, 8 Dec 2025 15:06:31 +0100 Subject: [PATCH 7/7] Add test case for resets without the NewSeqNo tag --- .gitignore | 3 +- crates/hotfix/src/session.rs | 4 ++ crates/hotfix/tests/common/test_messages.rs | 11 +++++ .../received_reset_tests.rs | 43 ++++++++++++++++++- 4 files changed, 59 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 2062358..a2fcb9d 100644 --- a/.gitignore +++ b/.gitignore @@ -5,4 +5,5 @@ *.db /data/* flamegraph.svg -tracing* \ No newline at end of file +tracing* +lcov.info \ No newline at end of file diff --git a/crates/hotfix/src/session.rs b/crates/hotfix/src/session.rs index 4026aa2..eed7bc7 100644 --- a/crates/hotfix/src/session.rs +++ b/crates/hotfix/src/session.rs @@ -457,6 +457,10 @@ impl, M: FixMessage, S: MessageStore> Session { .session_reject_reason(SessionRejectReason::RequiredTagMissing) .text("missing NewSeqNo tag in sequence reset message"); self.send_message(reject).await; + + // note: we don't increment the target seq number here + // this is an ambiguous case in the specification, but leaving the + // sequence number as is feels the safest return Ok(()); } }; diff --git a/crates/hotfix/tests/common/test_messages.rs b/crates/hotfix/tests/common/test_messages.rs index e64c516..b90a917 100644 --- a/crates/hotfix/tests/common/test_messages.rs +++ b/crates/hotfix/tests/common/test_messages.rs @@ -395,3 +395,14 @@ pub fn build_invalid_resend_request( msg.encode(&Config::default()).unwrap() } + +pub fn build_sequence_reset_without_new_seq_no(msg_seq_num: u64) -> Vec { + let mut msg = Message::new("FIX.4.4", "4"); // MsgType 4 = SequenceReset + msg.set(fix44::SENDER_COMP_ID, COUNTERPARTY_COMP_ID); + msg.set(fix44::TARGET_COMP_ID, OUR_COMP_ID); + msg.set(fix44::MSG_SEQ_NUM, msg_seq_num); + msg.set(fix44::SENDING_TIME, Timestamp::utc_now()); + // Deliberately omit NEW_SEQ_NO to create an invalid SequenceReset + + msg.encode(&Config::default()).unwrap() +} diff --git a/crates/hotfix/tests/session_test_cases/received_reset_tests.rs b/crates/hotfix/tests/session_test_cases/received_reset_tests.rs index cb66127..57ff740 100644 --- a/crates/hotfix/tests/session_test_cases/received_reset_tests.rs +++ b/crates/hotfix/tests/session_test_cases/received_reset_tests.rs @@ -9,7 +9,7 @@ use crate::common::actions::when; use crate::common::assertions::{assert_msg_type, then}; use crate::common::cleanup::finally; use crate::common::setup::given_an_active_session; -use crate::common::test_messages::TestMessage; +use crate::common::test_messages::{TestMessage, build_sequence_reset_without_new_seq_no}; use hotfix_message::fix44::MsgType; use tokio::test; @@ -99,3 +99,44 @@ async fn test_reset_moving_sequence_number_back_is_rejected() { finally(&session, &mut counterparty).disconnect().await; } + +/// Tests that receiving a SequenceReset without the required `NewSeqNo` field +/// results in a Reject message being sent. +/// +/// In this case, our target sequence number is not incremented. +/// This is an ambiguous area in the specification, but we think this option +/// is the safest of handling invalid resets. +#[test] +async fn test_reject_sequence_reset_without_new_seq_no() { + let (mut session, mut counterparty) = given_an_active_session().await; + + // the counterparty sends a SequenceReset without the required NewSeqNo field + let sequence_number = counterparty.next_target_sequence_number(); + let invalid_reset = build_sequence_reset_without_new_seq_no(sequence_number); + when(&mut counterparty) + .sends_raw_message(invalid_reset) + .await; + + // the session rejects this invalid SequenceReset with a Reject message + then(&mut counterparty) + .receives(|msg| { + assert_msg_type(msg, MsgType::Reject); + }) + .await; + + // verify the session remains active by sending a valid message + // note: since the invalid reset was rejected, the target sequence number wasn't incremented + // we need to delete the invalid reset from the counterparty's store first to send + // message with the same sequence number + counterparty.delete_last_message_from_store(); + + let sequence_number = counterparty.next_target_sequence_number(); + when(&mut counterparty) + .sends_message(TestMessage::dummy_execution_report()) + .await; + then(&mut session) + .target_sequence_number_reaches(sequence_number) + .await; + + finally(&session, &mut counterparty).disconnect().await; +}