-
Notifications
You must be signed in to change notification settings - Fork 71
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
FunctionGatewayV2 #204
FunctionGatewayV2 #204
Conversation
contracts/src/FunctionGatewayV2.sol
Outdated
mapping(bytes32 => address) functionIdToVerifier; | ||
|
||
function callbackRequestV1( | ||
bytes32 _functionId, |
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.
I feel like a better name for this function is something like requestWithContextV1
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.
and likewise fulfillRequestWithContextV1
contracts/src/FunctionGatewayV2.sol
Outdated
delete requests[_requestId]; | ||
} | ||
|
||
function storeRequestV1( |
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.
if most users are going to use this, we should just call it requestV1
imo
contracts/src/FunctionGatewayV2.sol
Outdated
} | ||
} | ||
|
||
function verifyStored( |
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.
the name of this function should be one word, like verify
or check
or something
contracts/src/FunctionGatewayV2.sol
Outdated
// Default case | ||
oracle[_functionId][inputHash] = outputHash; | ||
|
||
if (_address != address(0)) { |
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.
IMO we should have the callback functionality in a separate function rather than shove it in here
contracts/src/FunctionGatewayV2.sol
Outdated
revert CallbackFailed(_callbackAddress, _callbackSelector); | ||
} | ||
|
||
delete requests[_requestId]; |
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.
Not relevant to the changes but please remember to delete requests[_requestId];
before the .call
return; | ||
} | ||
|
||
VerifiedCall currentVerifiedCall = VerifiedCall({ |
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.
I'm really surprised if saving currentVerifiedCall
to memory like this works, pretty sure it needs to be a storage slot.
No description provided.