Skip to content

Move order outside market log to callsites#4090

Merged
jmg-duarte merged 1 commit intomainfrom
jmgd/log
Jan 28, 2026
Merged

Move order outside market log to callsites#4090
jmg-duarte merged 1 commit intomainfrom
jmgd/log

Conversation

@jmg-duarte
Copy link
Copy Markdown
Contributor

Description

The log inside the unwrap does not provide an actionable info, the lack or order ID, from address, quote ID, make it extremely hard to follow up on.

More context in https://cowservices.slack.com/archives/C0375NV72SC/p1769440848303459

Changes

  • Remove the log from the unwrap
  • Place it in the (seemingly) more relevant callsites

How to test

NA

@jmg-duarte jmg-duarte requested a review from a team as a code owner January 26, 2026 16:04
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

The pull request successfully moves logging statements to more relevant callsites, which improves the context of the logs. However, one change removes a warning log entirely when a market price check fails, which could hide potential arithmetic errors and make debugging more challenging. This review includes a suggestion to re-introduce a more informative warning log for such cases.

Comment thread crates/shared/src/order_validation.rs
@jmg-duarte jmg-duarte added this pull request to the merge queue Jan 28, 2026
Merged via the queue into main with commit 3fefcc0 Jan 28, 2026
19 checks passed
@jmg-duarte jmg-duarte deleted the jmgd/log branch January 28, 2026 15:13
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants