-
Notifications
You must be signed in to change notification settings - Fork 9
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
Fix review - removes TOKEN_ADDRESS_LIMIT logic in favor of a simple token whitelist #113
Conversation
…g the Ongoing+TieredPercentage implementations, interfaces, and methods in OpenQV1 and ClaimManagerV1
…quires token is whitelisted, or reverts
…ut a token address limit
…DDRESS_LIMIT, simply checks if token is whitelisted or not
…RESS_LIMIT in constructor
…RESS_LIMIT in the constructor
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.
Changes look good.
Token requirement has been simplified to just a whitelist. It makes the rest changes to support this revision. tokenAddressLimitReach method has been removed from DepositManagerV1. _tokenAddressLimit has been removed from TokenWhitelist constructor. TOKEN_ADDRESS_LIMIT has been removed along with it's setter. Tests have been updated to accommodate changes.
Removes the following storage variables from
TokenWhitelist.sol
:TOKEN_ADDRESS_LIMIT
Removes the following methods from
TokenWhitelist.sol
:setTokenAddressLimit
Removes the following parameters from
OpenQTokenWhitelist::constructor
:uint256 _tokenAddressLimit
Removes the following methods from
DepositManagerV1.sol
:tokenAddressLimitReached
Replaces the double-requires check for whitelist and
tokenAddressLimitReached
in favor of a simple requiresUpdates
DepositManager.test.js
,ClaimManager.test.js
,OpenQTokenWhitelist.test.js
andOpenQ.test.js
to initialize the OpenQTokenWhitelist without token address limitUpdates deploy script to properly deploy OpenQTokenWhitelist without token address limit
NOTE: It is now up to the protocol admin (OpenQ owners) to ensure the whitelist doesn't grow so large that it would cause an out of gas error on claim. We will only be adding a few whitelisted tokens