Problem
reader::is_position_liquidatable (contracts/reader/src/lib.rs:632-663) reimplements the same liquidation check as liquidation_handler::check_liquidatable — both resolve the position via OrderHandlerClient::get_position, load market props, fetch index/collateral prices from the oracle, and call the shared gmx_position_utils::is_liquidatable. liquidation_handler::check_liquidatable has direct test coverage (liquidate_underwater_long_removes_position_key, liquidate_underwater_short_removes_position_key both call it). reader::is_position_liquidatable has none:
grep -rn "is_position_liquidatable" --include="*.rs" .
# only the declaration at contracts/reader/src/lib.rs:632
Why it matters
This is the query path a UI or monitoring tool would call to show "is this position liquidatable" without needing to know about liquidation_handler. Since it's a separate, hand-duplicated implementation (not a call-through to liquidation_handler::check_liquidatable), any future change to one but not the other — e.g. adjusting price freshness handling, or which price bound is used — would silently make the two report different answers for the same position, and nothing would catch the divergence.
Suggested fix
Add a test mirroring liquidation_handler's own liquidation tests: open a position, crash the price, and assert reader::is_position_liquidatable returns true/false in lockstep with liquidation_handler::check_liquidatable for the same inputs. Consider having one call through to the other instead of maintaining two independent implementations.
Problem
reader::is_position_liquidatable(contracts/reader/src/lib.rs:632-663) reimplements the same liquidation check asliquidation_handler::check_liquidatable— both resolve the position viaOrderHandlerClient::get_position, load market props, fetch index/collateral prices from the oracle, and call the sharedgmx_position_utils::is_liquidatable.liquidation_handler::check_liquidatablehas direct test coverage (liquidate_underwater_long_removes_position_key,liquidate_underwater_short_removes_position_keyboth call it).reader::is_position_liquidatablehas none:Why it matters
This is the query path a UI or monitoring tool would call to show "is this position liquidatable" without needing to know about
liquidation_handler. Since it's a separate, hand-duplicated implementation (not a call-through toliquidation_handler::check_liquidatable), any future change to one but not the other — e.g. adjusting price freshness handling, or which price bound is used — would silently make the two report different answers for the same position, and nothing would catch the divergence.Suggested fix
Add a test mirroring
liquidation_handler's own liquidation tests: open a position, crash the price, and assertreader::is_position_liquidatablereturnstrue/falsein lockstep withliquidation_handler::check_liquidatablefor the same inputs. Consider having one call through to the other instead of maintaining two independent implementations.