refactor: access module #53
Conversation
Lets see if this little change makes our lives happier :)
solidity/contracts/Oracle.sol
Outdated
| IDisputeModule(_request.disputeModule).finalizeRequest(_request, _response, _accessControl.user); | ||
| IResponseModule(_request.responseModule).finalizeRequest(_request, _response, _accessControl.user); | ||
| IRequestModule(_request.requestModule).finalizeRequest(_request, _response, _accessControl.user); | ||
| IAccessModule(_request.accessModule).finalizeRequest(_request, _response, _accessControl.user); |
There was a problem hiding this comment.
I think there should be a zero address check at least for the access module here. I guess the others don't have it because they are expected to always exist.
Also, shouldn't it be set as an allowedModule at the time of creating a request, as happens with any other module?
There was a problem hiding this comment.
There was a discussion about this with Shaito over voice. I don't recall the specific argument, but we decided not to include it in the allowed module list/mapping.
One argument could be that allowedModules is used by modules to determine whether one module calling another is allowed (i.e. set during the request creation) by the Oracle.
An access module, although is not restricted, shouldn't be making external calls to other modules, therefore no other module should need to check whether the module is authorized.
we can move the discussion to this PR's base
There was a problem hiding this comment.
Even though there's no immediate use case in the context of Prophet, other protocols or external agents may be served with the possibility of asserting on-chain whether an access module is allowed for a particular oracle request—this is why I think it would increase composability.
🤖 Linear
Closes OPT-514