-
Notifications
You must be signed in to change notification settings - Fork 75
feat: Change RateModelStore => ConfigStore and add fields #150
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
Conversation
mrice32
left a comment
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.
Looks great! Only one minor comment
contracts/TokenConfigStore.sol
Outdated
| event UpdatedRateModel(address indexed l1Token, string rateModel); | ||
| event UpdatedTransferThreshold(address indexed l1Token, uint256 transferThreshold); | ||
|
|
||
| /** | ||
| * @notice Updates rate model string for L1 token. | ||
| * @param l1Token the l1 token rate model to update. | ||
| * @param rateModel the updated rate model. | ||
| */ | ||
| function updateRateModel(address l1Token, string memory rateModel) external onlyOwner { | ||
| l1TokenRateModels[l1Token] = rateModel; | ||
| emit UpdatedRateModel(l1Token, rateModel); | ||
| } | ||
|
|
||
| /** | ||
| * @notice Updates token transfer threshold percentage for L1 token. | ||
| * @param l1Token the l1 token rate model to update. | ||
| * @param transferThresholdPct the updated transfer threshold percentage. | ||
| */ | ||
| function updateTransferThreshold(address l1Token, uint256 transferThresholdPct) external onlyOwner { | ||
| l1TokenTransferThresholds[l1Token] = transferThresholdPct; | ||
| emit UpdatedTransferThreshold(l1Token, transferThresholdPct); | ||
| } |
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.
Why not just have a single config string to contain the rate model fields and the transfer threshold? That way we can add more token specific config later if needed?
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.
Great point, done
Adds constants from UMIP to
ConfigStorecontract.