Fix haircut logic#4093
Conversation
There was a problem hiding this comment.
Code Review
The pull request effectively addresses the critical bug where the driver-reported sell_amount could exceed the user's signed amount due to haircut logic. The changes correctly separate the haircut application for scoring purposes from the actual executed amounts, ensuring that sell_amount() returns only executed + fee. The introduction of haircut_in_sell_token() and Jit::custom_prices() are well-implemented to support this new logic. The updated and new tests provide good coverage for both sell and buy orders with haircut, verifying the fix and ensuring correct behavior. No critical issues were found in this review.
MartinquaXD
left a comment
There was a problem hiding this comment.
The change makes sense to me. Rather than announcing that a higher sell amount will leave the user's balance this only adjusts the custom prices.
Also the test where you assert that the autopilot is able to index the settlement correctly gives me high confidence that this is correct as the autopilot complained for the same reason the circuit breaker did. 👍
Description
The haircut feature had a critical bug where the driver-reported
sell_amountwould exceed the user's signed one. For example:Changes
sell_amount()- Now returns only executed + fee, which is the actual amount that left the user's wallethaircut_in_sell_token()helper - Computes haircut amount converted to sell tokencustom_prices()- Applies haircut only for quotes/scoring purposes, making bids more conservative without affecting reported amountsJit::custom_prices()- JIT orders don't have a haircut (for now), so they use simple sell/buy amount derivationHow to test
Adjusted existing and added new tests that fail on the
mainbranch, but work with the fix.