Conversation
ElliotFriedman
force-pushed
the
feat/v2/psm
branch
from
303fcb1 to
4ddf2d7
Compare
| function setExchangeRateScaledBase(uint256 usdPerEth) public { | ||
| price = Decimal.D256({ value: usdPerEth }); |
There was a problem hiding this comment.
I think it would be easier to have this function take in two unscaled parameters and use a ratio maybe? Nbd
Joeysantoro
left a comment
Joeysantoro
left a comment
There was a problem hiding this comment.
Left a bunch of comments. I don't think this PR should include any other contracts mentioned in the TODO. I haven't reviewed unit tests yet
Joeysantoro
left a comment
Joeysantoro
left a comment
There was a problem hiding this comment.
Looks really good! Left a few more organizational comments
xklob
left a comment
xklob
left a comment
There was a problem hiding this comment.
See comments; also, I don't see any admin role-assignment happening, unless I missed something?
Joeysantoro
left a comment
Joeysantoro
left a comment
There was a problem hiding this comment.
Reviewed one of the tests, assume the same comments apply to the ERC20 one.
Will review that one on my last pass. Last comment is the tests should look for events and event args more
There was a problem hiding this comment.
Matter of style, do you think duplicating the logic in the test is sufficient, or is it better to supply an exact amount so that we aren't missing the situation where both logics are incorrect?
I find it helpful to be explicit
There was a problem hiding this comment.
I can add tests where I calculate things out by hand before I run them and just use those constants in the tests.
There was a problem hiding this comment.
Add a test for after changing fee as well
There was a problem hiding this comment.
This test is confusing me, shouldn't end - start be equal to the surplus not the threshold? Also assert that the PSM holds exactly threshold at the end.
There was a problem hiding this comment.
No, start - end should equal the threshold as everything greater than the threshold got subtracted.
There was a problem hiding this comment.
check it does nothing when below threshold
There was a problem hiding this comment.
Why is this an ERC20 error?
There was a problem hiding this comment.
Because there is no approval done before redeem is called. This test isn't named well.
…replace with hasSurplus, add price range hook to internal getter methods
There was a problem hiding this comment.
Why can governance change the max?
There was a problem hiding this comment.
I can remove the governor's ability to do this. I added this on the off chance that we want to change the max fee in the future.
There was a problem hiding this comment.
I think it's fine to leave it in. Fine either way though.
There was a problem hiding this comment.
Remove .only
…ability to change max fee, floor and ceiling in price bound psm now get set in constructor
|
|
||
| /// @notice helper function to determine if price is within a valid range | ||
| function _validPrice(Decimal.D256 memory price) internal view returns (bool valid) { | ||
| valid = price.greaterThan(Decimal.ratio(floor, Constants.BASIS_POINTS_GRANULARITY)) |
There was a problem hiding this comment.
Should we use GTE & LTE here, instead of just GE & LE?
3 participants
The PegStabilityModule is a contract that combines the ReserveStabilizer and the BondingCurve contract into a single smart contract. All excess reserves will be sent to the Balancer pool to earn yield.
There are two different PSM's, one is for DAI and is called the PriceBoundPSM, and the other is for ETH. The DAI PriceBoundPSM has a feature that stops all swapping when the the price of DAI is above $1.02 or below $0.98. This feature protects FEI from a worst case scenario where DAI loses its peg. When the DAI price is above $1.02 or below $0.98, buying and selling will be paused until the price returns to the acceptable range.
The ETH PSM allows users to buy and sell FEI denominated in ETH. This module has no oracle safety feature.
Both contracts can have minting and redeeming paused by guardian or governor.
Done