Skip to content
Open
Changes from all commits
Commits
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
61 changes: 54 additions & 7 deletions src/app/dispute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,22 +168,28 @@ pub async fn dispute_action(
// Create new dispute record
let dispute = Dispute::new(order_id, order.status.clone());

// Setup dispute
// Setup dispute. In-memory only, so a rejection here writes nothing.
order
.setup_dispute(is_buyer_dispute)
.map_err(MostroCantDo)?;
order
.clone()
.update(pool)
.await
.map_err(|cause| MostroInternalErr(ServiceError::DbAccessError(cause.to_string())))?;

// Save dispute to database
// Row first, then the status flip — the ordering `job_escrow_deadline`
// already documents and relies on: a dispute row on a still-`active` or
// `fiat-sent` order is recoverable (its pass finishes the transition),
// whereas a `dispute` order with no row is invisible to solvers and,
// because `get_valid_order` admits only `Active`/`FiatSent`, can no
// longer be disputed by *either* party.
let dispute = dispute
.create(pool)
.await
.map_err(|cause| MostroInternalErr(ServiceError::DbAccessError(cause.to_string())))?;

order

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A successful Dispute::create followed by a failed order.update now leaves an initiated row plus an active order. That state is not safely recoverable: dispute_action permits Active, retries hit DisputeAlreadyExists, and the deadline worker resumes existing rows only in its FiatSent branch; its Active branch cancels the escrow. Make the writes atomic or add recovery before the active cancellation path, with failure-injection tests for both Active and FiatSent.

.clone()
.update(pool)
.await
.map_err(|cause| MostroInternalErr(ServiceError::DbAccessError(cause.to_string())))?;

// Get pubkeys of initiator and counterpart
let (initiator_pubkey, counterpart_pubkey) = if is_buyer_dispute {
(
Expand Down Expand Up @@ -733,4 +739,45 @@ mod tests {
let dispute = find_dispute_by_order_id(&pool, order.id).await.unwrap();
assert_eq!(dispute.status, DisputeStatus::Settled.to_string());
}

/// The `disputes` insert is made to fail deterministically by dropping the
/// table, so this pins the write ordering rather than any timing: the order
/// must not carry the dispute flag (nor the `Dispute` status) unless the
/// row that makes the dispute visible was written first.
#[tokio::test]
async fn dispute_action_leaves_the_order_untouched_when_the_dispute_row_fails() {
let pool = create_test_pool().await;
let ctx = build_ctx(&pool);
let buyer = Keys::generate().public_key();
let seller = Keys::generate().public_key();

let order = create_order(Some(buyer), Some(seller), Status::Active)
.create(&pool)
.await
.unwrap();

sqlx::query("DROP TABLE disputes")
.execute(&pool)
.await
.unwrap();

let event = create_event(buyer);
let result = dispute_action(
&ctx,
dispute_msg_for(Some(order.id)),
&event,
&Keys::generate(),
)
.await;

assert!(matches!(
result,
Err(MostroInternalErr(ServiceError::DbAccessError(_)))
));

let stored_order = Order::by_id(&pool, order.id).await.unwrap().unwrap();
assert!(!stored_order.buyer_dispute);
assert!(!stored_order.seller_dispute);
assert_eq!(stored_order.status, Status::Active.to_string());
}
}