[Separate Reporter Part 5] Implement deviation reporter #1900
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughWalkthroughThis update introduces significant improvements across several components in the Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Reporter
participant DataProcessor
participant WSHandler
App->>Reporter: create NewReporter(JobType)
Reporter->>App: store SubmissionPairs
App->>WSHandler: handleWsMessage(data)
WSHandler->>DataProcessor: ProcessDalWsRawData(data)
DataProcessor-->>WSHandler: return SubmissionData
WSHandler-->>App: send processed data
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
|
@coderabbitai review |
Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- node/pkg/reporter/app.go (4 hunks)
- node/pkg/reporter/reporter.go (5 hunks)
- node/pkg/reporter/types.go (4 hunks)
- node/pkg/reporter/utils.go (4 hunks)
Additional comments not posted (11)
node/pkg/reporter/types.go (3)
48-48: Approve the change toLastSubmissionfield type.Changing the
LastSubmissionfield type fromint32toint64increases the range of values it can store, which is beneficial for handling larger values.
137-140: Verify the impact of changingProofandFeedHashfields tostring.Changing the
ProofandFeedHashfields from[]byteand[32]bytetostringmay simplify serialization and deserialization processes. However, ensure that these changes do not introduce issues if the data is expected to be in byte format.
127-127: Approve the addition of the comment forLatestDatafield.Adding the comment indicating the mapping structure of the
LatestDatafield improves code readability and clarifies its intended use.node/pkg/reporter/app.go (3)
47-52: Approve the streamlined handling ofchainHelper.The changes simplify resource management by eliminating the need for a temporary variable and its associated cleanup.
93-106: Approve the integration ofdeviationReporterlogic.The changes ensure proper instantiation and integration of the
deviationReporter. Verify that the new logic does not introduce any issues.
171-176: Approve the enhanced data processing logic inhandleWsMessage.The changes improve data integrity and handling by validating or transforming the data before storing it. Verify the correctness of the
ProcessDalWsRawDatafunction.node/pkg/reporter/reporter.go (4)
38-38: Approve the addition ofJobTypefield toReporterstruct.The addition of the
JobTypefield allows for more dynamic behavior based on job types, enhancing the functionality of theReporterclass.
59-73: Approve the conditional logic based onJobTypeinRunmethod.The changes enhance the control flow by allowing the reporter to execute different methods based on the
JobType, improving the flexibility and maintainability of the code.
Line range hint
78-162: Approve the changes to thereportmethod.The changes allow for more flexible handling of submission data by processing any set of pairs passed to it, enhancing the functionality of the method.
164-177: Approve the addition ofdeviationJobmethod.The new method isolates the logic for handling deviations from standard reporting, enhancing the clarity and maintainability of the code.
node/pkg/reporter/utils.go (1)
Line range hint
60-73:
LGTM!The changes reflect a shift in dependency or data structure and the function handles contract reading and type assertions with error handling.
| @@ -78,78 +78,46 @@ func (a *App) setReporters(ctx context.Context) error { | |||
| WithInterval(groupInterval), | |||
| WithContractAddress(contractAddress), | |||
| WithCachedWhitelist(cachedWhitelist), | |||
| WithKaiaHelper(chainHelper), | |||
| WithLatestData(&a.LatestData), | |||
There was a problem hiding this comment.
let's define app.LatestData as *sync.Map type, and allocate memory from App.New() func. then, we can remove & here
There was a problem hiding this comment.
done! will be reflected in the next PR
* update last submission for deviation job * deviation reporter implemented * refactor data processing logic * remove retrier from deviation job * update feedHash and proof conversion * fix feedhash and proof conversion issue * clean up * reflect feedback * comment out app test
* update last submission for deviation job * deviation reporter implemented * refactor data processing logic * remove retrier from deviation job * update feedHash and proof conversion * fix feedhash and proof conversion issue * clean up * reflect feedback * comment out app test
* update last submission for deviation job * deviation reporter implemented * refactor data processing logic * remove retrier from deviation job * update feedHash and proof conversion * fix feedhash and proof conversion issue * clean up * reflect feedback * comment out app test
* update last submission for deviation job * deviation reporter implemented * refactor data processing logic * remove retrier from deviation job * update feedHash and proof conversion * fix feedhash and proof conversion issue * clean up * reflect feedback * comment out app test
* [Separate Reporter Part 1] Copy proof functions from reporter to dal (#1839) * copy proof related util functions from reporter to dal utils * check error in gopax to pass lint * rename import packages * placeholder for error return in gopax, pass lint * make proof functions lowercase in dal collector (#1841) * [Separate Reporter Part 2] base report logic with rest api (#1873) * comment out / remove unused code from reporter * comment out tests and create reporter entrypoint * temp comment out deviation reporter * initial dal fetch and submit logic * report logic implemented * temp disable reporter tests * remove reporter from node entrypoint * replace zerolog with zeropglog in reporter entrypoing * shared chain helper and mutex * clean up unused reporter utils (#1881) * [Separate Reporter Part 3] Websocket implementation (#1887) * initial dal fetch and submit logic * fix lint issues * replace const ticker time with submission interval * initial implementation of dal websocket * uncomment report delegated to pass linting * websocket implemented * remove manual dial from reporter app * reflect feedback * make reporting concurrent * remove unnecessary comments * resolve reporter conflicts * [Separate Reporter Part 5] Implement deviation reporter (#1900) * update last submission for deviation job * deviation reporter implemented * refactor data processing logic * remove retrier from deviation job * update feedHash and proof conversion * fix feedhash and proof conversion issue * clean up * reflect feedback * comment out app test * [Separate Reporter Part 6] implement tests (#1902) * implement main and app test * refactor latest data * fetch configs from admin and save in local db * reporter and util tests * handle edge cases utils_test * uncomment reporter test in taskfile * remove reporter from admin * give more timeout for fethcing configs * remove hardcoded test name from taskfile * replace config fetching with mock data * reflect feedback * fix faulty deviation reporter logic and udpate tests * refactor regular and deviation reporter job functions * rename latest data variable names * rename reporter opt functions * rename GetLatestData param name * reporter health check endpoint and sentinel check (#1912) * reporter health endpoint * include reporter health check to sentinel * move healthcheck creation from run to cmd * remove sentinel updates * fetch configs in reporter (#1915) * replace db config query logic with fetch * fix test issue * remove db from tests * remove db from config struct * remove count from taskfile * remove duplicate log * fix go lint issue * reflect feedback * add error log in deviation reporter * get dal api key from secrets
Description
Please include a summary of the changes and the related issue.
Please also include relevant motivation and context.
List any dependencies that are required for this change.
Related issue #1777
Type of change
Please delete options that are not relevant.
Checklist before requesting a review
Deployment