Skip to content

chore: include offending order UID in fulfillment error message#4415

Open
edjroz wants to merge 2 commits into
cowprotocol:mainfrom
edjroz:chore/fulfillment-error-uid
Open

chore: include offending order UID in fulfillment error message#4415
edjroz wants to merge 2 commits into
cowprotocol:mainfrom
edjroz:chore/fulfillment-error-uid

Conversation

@edjroz
Copy link
Copy Markdown

@edjroz edjroz commented May 15, 2026

Description

When Solutions::into_domain in crates/driver/src/infra/solver/dto/solution.rs rejects a fulfillment whose order UID isn't in the auction, the error string was generic:

invalid order UID specified in fulfillment

The UID itself was dropped. Since this error is surfaced to solver operators via the notify mechanism as a DeserializationError, operators had no way to tell which of their fulfillments was rejected. A // TODO this error should reference the UID comment on line 52 already flagged this gap.

Changes

  • Extract the order lookup into a small find_order helper.
  • Include the hex-encoded UID in the error message:
    invalid order UID specified in fulfillment: 0xabab…ab.
  • Drop the resolved TODO.
  • Add a focused unit test that asserts the offending UID appears in the
    message.

How to test

  1. cargo test -p driver --lib fulfillment_unknown_uid_error_includes_uid

Related Issues

Fixes #4414

@edjroz edjroz requested a review from a team as a code owner May 15, 2026 14:47
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the order lookup logic into a dedicated find_order helper function, which now includes the order UID in error messages for better traceability. A unit test was also added to ensure the error message correctly contains the UID. No critical issues found.

Comment on lines +296 to +300
let uid: competition::order::Uid = uid.into();
super::Error(format!(
"invalid order UID specified in fulfillment: {}",
uid.0
))
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.

Won't uid.0 be rendered in hex and as such couldn't we just do:

Suggested change
let uid: competition::order::Uid = uid.into();
super::Error(format!(
"invalid order UID specified in fulfillment: {}",
uid.0
))
super::Error(format!(
"invalid order UID specified in fulfillment: {}",
uid.0
))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore (driver): include offending order UID in fulfillment error message

2 participants