Conversation
|
I know it's not merged yet, but if you feel like spending an afternoon with a pen & paper, would be nice to derive the formula for resistant balance of the Curve pool too |
|
Made wrapper to adapt existing PCV deposits without redeploying: #156 |
Pcv deposit wrapper
| /// @notice returns a manipulation resistant account of both the balance of underlying and protocol owned FEI | ||
| /// @dev to allow compatibility with the Collateralization Oracle even though this contract isn't an IPCVDeposit | ||
| function resistantBalanceAndFei() public view returns(uint256, uint256) { | ||
| return (balance(), 0); |
There was a problem hiding this comment.
I wonder if protocol contracts should report their current balance of FEI instead of 0. Like there could be a weird attack where FEI is sent to the protocol contracts, and the protocol believes it is undercollateralized & mint TRIBE, where actually it is holding the FEI. At first glance this looks like an attack that is a loss for the attacker, but idk there may be some wierd interaction somewhere ?
There was a problem hiding this comment.
+1 this is definitely something to put some thought into - though, a special version of the Bonding Curve that implements logic susceptible to this kind of attack could just override this method.
If anything, it's better to leave it as the default of zero - I cold see an attack far more likely where someone sends FEI borrowed from a lending pool to the contract, has the contract count it, and double-counts PCV this way.
There was a problem hiding this comment.
Default of 0 makes most sense in this context because the FEI is likely held as a deposit receipt in Fuse or whatever. Alternatively we could add a distinct "Fei" PCV deposit that reports in-line with expectations for FEI
| /// @notice returns the resistant balance and FEI in the deposit | ||
| function resistantBalanceAndFei() public view returns (uint256, uint256) { | ||
| uint256 resistantBalance = balance(); | ||
| uint256 reistantFei = isProtocolFeiDeposit ? resistantBalance : 0; |
There was a problem hiding this comment.
So, FEI deposits add 1000 FEI to PCV and 1000 FEI to protocol-owned FEI, but we'll use an oracle price of 0 for FEI, so it will add 0$ to the PCV and 1000 FEI into circulation, correct ?
There was a problem hiding this comment.
^ this is confusing and should be very clearly spelled out in docs if we go forward with it
There was a problem hiding this comment.
@eswak 's comment was the intended behavior. Alternatively we can return 0 for the balance on Fei deposits to be explicit. @ditchfieldcaleb is the intended outcome confusing or just the implementation?
xklob
left a comment
xklob
left a comment
There was a problem hiding this comment.
Feedback submitted. I still want to review the derivation for the Uniswap resistant balance but I'm sure its fine if it's been checked over a few times.
3 participants
Finishes the work to update all PCVDeposit contracts by adding resistant balance calls
The ETH PCV deposits use the WETH address as balanceReportedIn because ETH doesn't have an address.
The only complex resistant balance calculation is uniswap, which has a derivation in the comments.
TODO: