Remove position wrapper methods from Pool#157
Conversation
Consolidate position health getters/setters and sink/source methods by having Position (layer 3) access InternalPosition directly via a new EPosition-scoped borrowPosition method, removing 8 thin pass-through methods from Pool (layer 2). - Move validation into InternalPosition setters (setMinHealth, setMaxHealth, setTargetHealth) so it lives in one place - Fix setMaxHealth precondition bug: was >= 1.0, now >= self.targetHealth (matching its error message) - Add EPosition-scoped borrowPosition, lockPosition, unlockPosition to Pool for use by Position resources - Rewrite Position health getters/setters and provideSink/provideSource to use borrowPosition directly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Test that setMinHealth rejects values below 1.0 - Test that withdrawals are rejected when they would drop health below 1.0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- setMinHealth rejects values above targetHealth - setTargetHealth rejects values at or below minHealth - setTargetHealth rejects values at or above maxHealth - setMaxHealth rejects values below targetHealth Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move set_min_health, set_target_health, set_max_health from test transactions to cadence/transactions/flow-alp/position/ and remove test-only warnings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jordanschalm
changed the title
| targetHealth > self.minHealth: "Target health (\(targetHealth)) must be greater than min health (\(self.minHealth))" | ||
| targetHealth < self.maxHealth: "Target health (\(targetHealth)) must be less than max health (\(self.maxHealth))" |
There was a problem hiding this comment.
These are changed to strict inequalities, but not the corresponding ones in SetMinHealth/SetMaxHealth? We should be consistent in whether we want min <= target <= max vs min < target < max.
There was a problem hiding this comment.
Good point, I think strict inequalities makes the most sense. 741e9a7
There was a problem hiding this comment.
interesting, i typically expect inclusive, since the "max a value can be" in my mind definitely includes that value. Anyway, i'm okay either way as long as it's consistent
There was a problem hiding this comment.
Yeah, I guess it depends on the context. In this case, a config of for exampleminHealth=targetHealth would result in us "rebalancing" a position directly into a state where it is eligible for rebalancing again. That's why strict inequality seems more appropriate to me in this case.
zhangchiqing
left a comment
zhangchiqing
left a comment
There was a problem hiding this comment.
LGTM. Also agree to tim's comments.
| targetHealth > self.minHealth: "Target health (\(targetHealth)) must be greater than min health (\(self.minHealth))" | ||
| targetHealth < self.maxHealth: "Target health (\(targetHealth)) must be less than max health (\(self.maxHealth))" |
There was a problem hiding this comment.
interesting, i typically expect inclusive, since the "max a value can be" in my mind definitely includes that value. Anyway, i'm okay either way as long as it's consistent
4 participants
This PR consolidates position health getters/setters and sink/source methods by having
PositionaccessInternalPositiondirectly via a newEPosition-scopedborrowPositionmethod, removing 8 thin pass-through methods fromPool. Previously there were 3 layers of functions for many position-related operations, that all did the same thing: this PR removes the middle layer where possible.This PR also adds constraints to
Position.minHealthto enforce that it is always >1. This has the side effect of fixing an edge case where a user could withdraw from an unhealthy position by manipulatingminHealth(this branch contains a test demonstrating the scenario).EPosition-scoped lock/unlock toPoolfor use byPositionresourcesborrowPositiondirectly