Closed crypto-vincent closed 1 year ago
This is too wild in terms of naming and quite confusing.
At this rate if we keep introducing new terms, helpers, not sure how can we maintain codebase in the future.
To provide more granular feedback:
* `let total_redeemable_amount_under_management = calculate_depositories_sum_value(&vec![` Not so total, a depository is left out (LSD), the helper function abstract away some weird ROUTER_DEPOSITORY_COUNT, add new weird error.
I can go on but idk can you feel it or now, cause I'm often commenting these.
I think this change is too small to warrant adding this amount of complexity/convolution to the codebase. Also no need to make something generic before we have 2 cases. We might never have another case and LSD might not even stick, premature imo.
I would rather do:
* keep the existing code * where it uses `controller.redeemable_circulating_supply` replace by a new variable that is `controller.redeemable_circulating_supply - lsd_depository.redeemable_under_management`, that you can put in a getter if you prefer
Wdyt
Apologies if this PR frustrated you, that clearly was not my intent.
This was the best solution I came up with based on the following reasoning:
So my reasoning was that I want to avoid having to inject the non-router depository into the router-related IXs so I went for using the sum of the router depositories (it's kinda like the router depositories are a pool of liquidity they share among themselves and not the other depositories).
This is to avoid having to inject the lds_depository account into the IXs:
Always happy to change any naming if need be, i'm not too sure what should I call those.
Do you think we should just inject the lds_depositories everywhere ?
could use some naming like routable_redeemable_circulating_supply
or redeemable_circulating_supply_under_router
?
Hey no worries.
Then why not add a new value in the controller that keep the circulating supply that accomodate the router? I think this simplify the whole PR more or less to that value + updating the calculation in the existing code? Wdyt
Pausing for now since LSD is changing plans
Update the mathematics for the router to not use
controller.redeemable_circulating_supply
This is because other non-router depositories may have an effect on that global value, which would skew the target computation results because the router only should balance the router's depositories redeemable, not outsiders depositories.