-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Add Rebasable token to Optimism #58
base: main
Are you sure you want to change the base?
Conversation
) external { | ||
for (uint256 obIndex = 0; obIndex < observers.length; obIndex++) { | ||
try ITokenRatePusher(observers[obIndex]).pushTokenRate() {} | ||
catch (bytes memory lowLevelRevertData) { |
Check warning
Code scanning / Slither
Uninitialized local variables Medium
contracts/lido/TokenRateNotifier.sol
Outdated
function observersLength() public view returns (uint256) { | ||
return observers.length; | ||
} |
Check warning
Code scanning / Slither
Public function that could be declared external Warning
- TokenRateNotifier.observersLength()
Add ERC-2612/EIP-1271 permit to rebasable token
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Second pass 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a note about deployment behind proxies for this and other relevant contracts
It would be great to have it as a part of the spec
contracts/token/ERC20Rebasable.sol
Outdated
WRAPPED_TOKEN = IERC20(wrappedToken_); | ||
TOKEN_RATE_ORACLE = ITokenRateOracle(tokenRateOracle_); | ||
BRIDGE = bridge_; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__disable_Initializers()
or whatever
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://forum.openzeppelin.com/t/what-does-disableinitializers-function-mean/28730
relevant mostly not for this contract, but for others (bridges, might be applicable for TokenRateOracle, whatever)
…tests, fix comments
WHAT
Implement a new rebasable token on Optimism
HOW
Detailed schemes and explanation you can find here https://hackmd.io/@lido/ryKP1ssm6
TODO