Skip to content

tweak handling of orders without signing address #546

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.

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

Merged
merged 1 commit into from
Apr 10, 2025

Conversation

dvush
Copy link
Contributor

@dvush dvush commented Apr 8, 2025

πŸ“ Summary

Better handle orders without signer address

πŸ’‘ Motivation and Context


βœ… I have completed the following steps:

  • Run make lint
  • Run make test
  • Added tests (if applicable)

@Copilot Copilot AI review requested due to automatic review settings April 8, 2025 17:00
@dvush dvush requested review from ZanCorDX and ferranbt as code owners April 8, 2025 17:00
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

crates/rbuilder/src/backtest/redistribute/mod.rs:506

  • [nitpick] Changing the log level from warn to error could lead to excessive error logging if orders missing redistribution addresses are expected in some scenarios. Please confirm that this behavior is intentional.
error!(order = ?order_id, "Included order redistribution address not found");

crates/rbuilder/src/backtest/redistribute/mod.rs:495

  • [nitpick] Consider renaming 'included_without_address' to a more descriptive name such as 'ordersMissingAddress' to clarify its purpose.
let mut included_without_address = HashSet::default();

@dvush dvush merged commit ddc9d9c into develop Apr 10, 2025
4 checks passed
@dvush dvush deleted the redistribution_fixed_2025_04_08 branch April 10, 2025 14:29
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.

2 participants