Add and test privileged rewards notifier role #12
Conversation
wildmolasses
changed the title
apbendi
force-pushed
the
notifier
branch
2 times, most recently
from
1274ad0
to
728ed1f
Compare
test/UniStaker.t.sol
Outdated
| view | ||
| returns (uint256 _boundedRewardAmount) | ||
| { | ||
| _boundedRewardAmount = bound(_rewardAmount, 200e6, 10_000_000e6); |
There was a problem hiding this comment.
nit: i don't love the lower bound of 200e6 because i don't know if it's suggested or enforced in the external contracts. in practice is it presumed to be higher than 200e6?
There was a problem hiding this comment.
Since we don't know the tokens that will be used for sure, it's hard to pick reasonable ranges here. You might say, "well shouldn't work for all values?". Unfortunately you can't do that, because like many DeFi protocols that involve any kind of ratio between token amounts, there's an embedded assumption about the "reasonable" ranges in which values will fall.
For many contracts, this one included, it's possible generate scenarios where the token(s) contract(s) chosen would break the protocol due to the proliferation of rounding errors that would occur. For example, tokens where HUGE balances or tiny balances might (i.e. literally 1 with no decimals) break things. And even if the protocol itself wouldn't break (because it rounds conservatively to favor the contract, it would make writing meaningful tests nearly impossible.
So, long way of saying, we can't really lower the bottom bound without breaking the 1% rounding assumption I baked into the test cases. But this got me thinking: we can open up the upper bound without breaking the tests, so I did.
FWIW, the reason I chose e6 here was to simulate USDC's ERC20, which is likely the lowest precision token that might actually be chosen at only 6 decimals. And 200 USDC is well below what the real threshold is liable to be set to. So for this particular project, the ranges chosen are probably safe to be considered realistic.
|
Coverage after merging notifier into stake-earn will be
Coverage Report
|
|||||||||||||||||||||||||
This PR makes the
notifyRewardsAmounta permissioned method which can only be updated by the an immutable rewards notifier that is set at deployment. It also adds a full set of tests for thenotifyRewardsAmountmethod, which was previously untested.