Add view function path for Data Streams llo-feeds Verifier contracts #309
Conversation
|
👋 cawthorne, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR adds view-only versions of verification functions to the Data Streams llo-feeds Verifier contracts. The new verifyView and verifyBulkView functions provide read-only access to report verification without emitting events or billing users, making them compatible with static calls and off-chain simulations.
Key Changes:
- Added
verifyViewandverifyBulkViewfunctions to bothIVerifierandIVerifierProxyinterfaces - Implemented view-only verification methods in
VerifierandVerifierProxycontracts - Refactored existing verification logic to share code between state-changing and view-only variants
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| contracts/src/v0.8/llo-feeds/v0.5.0/interfaces/IVerifierProxy.sol | Adds interface definitions for verifyView and verifyBulkView functions |
| contracts/src/v0.8/llo-feeds/v0.5.0/interfaces/IVerifier.sol | Adds interface definition for verifyView function |
| contracts/src/v0.8/llo-feeds/v0.5.0/VerifierProxy.sol | Implements view-only verification methods and bumps version to 2.0.1 |
| contracts/src/v0.8/llo-feeds/v0.5.0/Verifier.sol | Refactors verification logic into shared internal function and implements verifyView |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function _verify( | ||
| bytes calldata signedReport | ||
| ) internal view returns (bytes memory verifierResponse) { | ||
| if (msg.sender != i_verifierProxyAddr) revert AccessForbidden(); |
There was a problem hiding this comment.
The msg.sender check in _verify will fail for verifyView calls because msg.sender is only set during transactions, not view calls. This check should either be removed from the internal function and placed only in the state-changing verify function, or a sender parameter should be passed to _verify.
| if (msg.sender != i_verifierProxyAddr) revert AccessForbidden(); |
|
|
||
| /// @notice Internal verification logic shared by verify() and verifyView() | ||
| /// @param signedReport The signed report to verify | ||
| /// @return verifierResponse The verified report data |
There was a problem hiding this comment.
[nitpick] The return parameter name in the documentation ('verifierResponse') doesn't match the actual implementation which uses 'verifierResponse' in the function signature but the comment could be clearer that this returns the decoded report data.
| /// @return verifierResponse The verified report data | |
| /// @return verifierResponse The decoded report data extracted from the signed report |
Static analysis results are availableHey @cawthorne, you can view Slither reports in the job summary here or download them as artifact here. |
1 participant
Upgrading the LLO Feeds Verifier and Verifier Proxy to have a view function path.
Audit by dedaub:
https://docs.google.com/document/d/1zjdP_BjsHij5M76LaHfW3OoU1fbEaSj4njdcEq1syZ0/edit?usp=sharing