Skip to content

Fix #669: Prevent dangling ConnectionState from GetOffer messages #680

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
omsuneri wants to merge 2 commits into from
Closed

Fix #669: Prevent dangling ConnectionState from GetOffer messages #680

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4253fb8
Fix #669: Prevent dangling ConnectionState from GetOffer messages
omsuneri Jan 1, 2026
cb0122c
Fix #669: Prevent dangling ConnectionState from GetOffer messages
omsuneri Jan 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions src/maker/handlers2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,36 @@ use crate::protocol::messages2::{

/// The Global Handle Message function for taproot protocol. Takes in a [`Arc<Maker>`] and handles
/// messages according to the new taproot message flow without requiring state expectations.
/// returns a tuple of `(Option<`Response`>, should_persist)`
/// - response: the message to send back to taker
/// - should_persist: whether the connection_state should be saved to ongoing_swaps
pub(crate) fn handle_message_taproot(
maker: &Arc<Maker>,
connection_state: &mut ConnectionState,
message: TakerToMakerMessage,
) -> Result<Option<MakerToTakerMessage>, MakerError> {
) -> Result<(Option<MakerToTakerMessage>, bool), MakerError> {
log::debug!(
"[{}] Handling message: {:?}",
maker.config.network_port,
message
);

// Handle messages based on their type, not on expected state
match message {
TakerToMakerMessage::GetOffer(get_offer_msg) => {
handle_get_offer(maker, connection_state, get_offer_msg)
let response = handle_get_offer(maker, connection_state, get_offer_msg)?;
Ok((response, false))
}
TakerToMakerMessage::SwapDetails(swap_details) => {
handle_swap_details(maker, connection_state, swap_details)
let response = handle_swap_details(maker, connection_state, swap_details)?;
Ok((response, true))
}
TakerToMakerMessage::SendersContract(senders_contract) => {
handle_senders_contract(maker, connection_state, senders_contract)
let response = handle_senders_contract(maker, connection_state, senders_contract)?;
Ok((response, true))
}
TakerToMakerMessage::PrivateKeyHandover(privkey_handover_message) => {
handle_privkey_handover(maker, connection_state, privkey_handover_message)
let response = handle_privkey_handover(maker, connection_state, privkey_handover_message)?;
Ok((response, true))
}
}
}
Expand Down Expand Up @@ -82,14 +88,12 @@ fn handle_swap_details(
swap_details.no_of_tx
);

// Reject if GetOffer wasn't received first (my_privkey must be set)
// This ensures the taker has a fresh offer with a valid tweakable_point
if connection_state.incoming_contract.my_privkey.is_none() {
log::warn!(
"[{}] Rejecting SwapDetails - GetOffer must be sent first to establish keypair",
log::info!(
"[{}] Generating new keypair for SwapDetails",
maker.config.network_port
);
return Ok(Some(MakerToTakerMessage::AckResponse(AckResponse::Nack)));
let _ = maker.create_offer(connection_state)?;
}

// Reject if there's already an active swap in progress for this connection
Expand Down
102 changes: 60 additions & 42 deletions src/maker/server2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,65 +478,83 @@ fn handle_client_taproot(maker: &Arc<Maker>, stream: &mut TcpStream) -> Result<(

// Get or create connection state for this swap id
let mut connection_state = {
let mut ongoing_swaps = maker.ongoing_swap_state.lock()?;
let ongoing_swaps = maker.ongoing_swap_state.lock()?;

match &message {
TakerToMakerMessage::GetOffer(_) => {
// TODO: Right now sync_offerbook is sending GetOffer with an empty string swap_id
// Refactor to not create a connection state when swap id is an empty string
// Just return the Offer
let (state, _) = ongoing_swaps.entry(swap_id.clone()).or_insert_with(|| {
log::info!(
"[{}] Creating new connection state for {:?}",
maker.config.network_port,
&swap_id
);
(ConnectionState::default(), Instant::now())
});
state.clone()
log::debug!(
"[{}] Using temporary connection state for GetOffer from {:?}",
maker.config.network_port,
&swap_id
);
ConnectionState::default()
}
TakerToMakerMessage::SwapDetails(_) => {
match ongoing_swaps.get(&swap_id) {
Some((state, _)) => {
log::debug!(
"[{}] Found existing connection state for SwapDetails",
maker.config.network_port
);
state.clone()
}
None => {
log::debug!(
"[{}] Creating new connection state for SwapDetails",
maker.config.network_port
);
ConnectionState::default()
}
}
}
_ => match ongoing_swaps.get(&swap_id) {
Some((state, _)) => state.clone(),
None => {
log::info!(
"[{}] No connection state found for swap_id={:?}. GetOffer must be sent first.",
_ => {
match ongoing_swaps.get(&swap_id) {
Some((state, _)) => state.clone(),
None => {
log::warn!(
"[{}] No connection state found for swap_id={:?}. SwapDetails must be sent first.",
maker.config.network_port,
&swap_id
);
return Err(MakerError::General("No connection state found"));
return Err(MakerError::General(
"No connection state found. Send SwapDetails first.",
));
}
}
},
}
}
};

// Handle the message using taproot handlers
let response = match handle_message_taproot(maker, &mut connection_state, message) {
Ok(response) => response,
Err(e) => {
log::error!(
"[{}] Error handling message from {}: {:?}",
maker.config.network_port,
ip,
e
);
let (response, should_persist) =
match handle_message_taproot(maker, &mut connection_state, message) {
Ok(result) => result,
Err(e) => {
log::error!(
"[{}] Error handling message from {}: {:?}",
maker.config.network_port,
ip,
e
);

// Check if this is a behavior-triggered error
match &e {
MakerError::General(msg) if msg.contains("behavior") => {
log::info!(
"[{}] Behavior-triggered disconnection",
maker.config.network_port
);
// Check if this is a behavior-triggered error
match &e {
MakerError::General(msg) if msg.contains("behavior") => {
log::info!(
"[{}] Behavior-triggered disconnection",
maker.config.network_port
);
}
_ => {}
}
_ => {}
break;
}
break;
}
};
{
};

if should_persist {
let mut ongoing_swaps = maker.ongoing_swap_state.lock()?;
log::info!(
"[{}] Saving connection state for {}: swap_amount={}, my_privkey_is_some={}",
"[{}] Persisting connection state for {}: swap_amount={}, my_privkey_is_some={}",
maker.config.network_port,
ip,
connection_state.swap_amount,
Expand Down
Loading