-
Notifications
You must be signed in to change notification settings - Fork 0
fix[l-03]: check whitelist interface using erc165 #17
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
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
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.
Pull Request Overview
This PR implements interface validation using ERC165 for whitelist contracts to ensure type safety. The changes add ERC165 support to the whitelist interface hierarchy and validate that contracts implement the expected interface before assignment.
- Added ERC165 interface checking to validate whitelist contract compatibility
- Modified whitelist interfaces and implementations to inherit from IERC165 and support interface detection
- Updated validation logic to use ERC165Checker instead of simple zero address checks
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
ManagedOptimisticOracleV2.sol | Added ERC165 interface validation for whitelist assignments and removed zero address checks for internal setters |
AddressWhitelistInterface.sol | Extended interface to inherit from IERC165 for ERC165 support |
DisableableAddressWhitelist.sol | Implemented supportsInterface method to support ERC165 interface detection |
AddressWhitelist.sol | Added ERC165 inheritance and implemented supportsInterface method |
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
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.
This looks right, but hard to meaningfully give approval before we merge master into it (as e.g. DisableableWhitelistInterface
got removed)
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
// Zero address is allowed to disable the custom proposer whitelist. | ||
if (whitelist != address(0)) { | ||
require( | ||
ERC165Checker.supportsInterface(whitelist, type(AddressWhitelistInterface).interfaceId), |
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.
nit: maybe this could go in a internal view function "_validateWhitelist" given that we check this in different places.
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.
indeed, great suggestion, added in 7bb517c
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 good, just an small suggestion
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
This should be merged after #6 resolving for any conflicts and also adding
supportsInterface
implementation to the newDisabledAddressWhitelist
contract.