OA tx : Balancer pfei accounting fix + remove PCVDeposits with bad debts#776
OA tx : Balancer pfei accounting fix + remove PCVDeposits with bad debts#776Joeysantoro merged 15 commits intodevelopfrom
Conversation
thomas-waite
left a comment
thomas-waite
left a comment
There was a problem hiding this comment.
Looks good to me. I'd probably add a unit test to validate the fix, but overall looks great
proposals/description/oa_cr_fix.ts
Outdated
| target: 'collateralizationOracle', | ||
| values: '0', | ||
| method: 'swapDeposit(address,address)', | ||
| arguments: ['{balancerLensBpt30Fei70Weth}', '{balancerLensBpt30Fei70WethFixed}'], |
There was a problem hiding this comment.
now is a good time to rethink our naming conventions. The arbitrary suffix "fixed" is not a super scalable convention.
If it has the same ABI I'd prefer making the fixed one called balancerLensBpt30Fei70Weth and naming the old one "old" or
"v1" or something
There was a problem hiding this comment.
Yeah would prefer version numbers over "fixed"
Reminds me of how I used to name files in folders for school assignments. "Assignment1Fixed", "Assignment1FixedFinal", "Assignment1FixedFinalFinalSubmitThis.doc"
There was a problem hiding this comment.
renamed the old one to balancerLensBpt30Fei70WethOld and use the balancerLensBpt30Fei70Weth name for the new one.
|
Updated the PR description / screenshot / calldata, I think we're good to ship |
4 participants
OA Action to fix CR accounting
Accounted PCV loss :
Accounted protocol FEI becoming circulating :
About the Balancer lens fix :
It used to report all the FEI in the B-70WETH-30FEI pool as protocol-owned (28M) and not only the FEI that is within the LP tokens held by the protocol. The protocol owns ~70% of the pool, so it used to count 28M porotocol FEI (the whole pool) instead of 19M (70% of total).
New lens address : https://etherscan.io/address/0x673f7dfa863b611de657759aede629b260f4e682