feat: mutable unlockers controller and factory #9
Conversation
Signed-off-by: Matt Rice <matthewcrice32@gmail.com>
Signed-off-by: chrismaree <christopher.maree@gmail.com>
Signed-off-by: chrismaree <christopher.maree@gmail.com>
| @@ -14,7 +14,7 @@ contract TestImmutableController is ImmutableController, MockSourceAdapter, Base | |||
| {} | |||
| } | |||
|
|
|||
| contract OvalUnlockLatestValue is CommonTest { | |||
| contract ImmutableControllerTest is CommonTest { | |||
There was a problem hiding this comment.
I fixed this. the naming was wrong and we had silly tests in here that dont make sence.
mrice32
changed the title
Signed-off-by: Matt Rice <matthewcrice32@gmail.com>
Signed-off-by: Matt Rice <matthewcrice32@gmail.com>
Co-authored-by: Reinis Martinsons <77973553+Reinis-FRP@users.noreply.github.com> Co-authored-by: Pablo Maldonado <pablo@umaproject.org>
| revert SenderNotApproved(msg.sender); | ||
| } | ||
|
|
||
| (bool success, bytes memory returnData) = target.call{value: value}(callData); |
There was a problem hiding this comment.
Why do we need generic execute with low level call? If this is intended only to extend unlocker permission, why not simplify it here to only forward the unlockLatestValue call to target oval instance?
There was a problem hiding this comment.
I just figured it would be better to make it more general in case we wanted to use this functionality elsewhere.
For instance, if there were a redstone feed that only Oval was allowed to push prices to, this might be useful there as well.
src/factories/BaseFactory.sol
Outdated
| ); | ||
|
|
||
| constructor(uint256 maxTraversal, address[] memory _defaultUnlockers) { | ||
| MAX_TRAVERSAL = maxTraversal; |
There was a problem hiding this comment.
Should we call it defaultMaxTraversal and have a setter for this?
There was a problem hiding this comment.
I don't really see max traversal changing that often for a particular oracle type. If it were to change, I think it would be acceptable to just move to a new factory.
| * for the Oval auction to be run and, thus, the maximum time that prices will be delayed. | ||
| * @return oval deployed oval address. | ||
| */ | ||
| function create(IAggregatorV3Source source, uint256 lockWindow) external returns (address oval) { |
There was a problem hiding this comment.
Do you think there could be cases where someone wants to create an Oval instance with a different list of unlockers than the defaultUnlockers or max traversal?
There was a problem hiding this comment.
I think we want official Oval deployments to have the same unlocker setup, whereby default unlockers managed by Risk Labs are used.
I don't think these factories are intended for nonstandard deployments, but you can always do a custom deployment if you want that!
Signed-off-by: Matt Rice <matthewcrice32@gmail.com>
This PR includes: