-
Notifications
You must be signed in to change notification settings - Fork 15
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
(OraklNode) Deviation Threshold based on submission interval #1767
(OraklNode) Deviation Threshold based on submission interval #1767
Conversation
WalkthroughWalkthroughThe recent changes enhance the deviation reporting capabilities of the system by introducing a dynamic deviation threshold, which is now calculated based on the submission interval. Additionally, constants such as deviation intervals and reporting thresholds have been adjusted. These improvements are reflected in both the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Reporter
participant Utils
Client->>Reporter: Initialize via NewReporter()
Reporter->>Utils: Call GetDeviationThreshold(interval)
Utils-->>Reporter: Return calculated threshold
Reporter->>Client: Return initialized Reporter instance
Reporter->>Utils: Call GetDeviatingAggregates(data, deviationThreshold)
Utils-->>Reporter: Return deviating aggregates
Reporter->>Client: Report deviations
Poem
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 Configration File (
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- node/pkg/reporter/reporter.go (2 hunks)
- node/pkg/reporter/types.go (2 hunks)
- node/pkg/reporter/utils.go (4 hunks)
Additional comments not posted (8)
node/pkg/reporter/types.go (3)
31-32
: Verify the new constant values.The
MAX_REPORT_BATCH_SIZE
has been reduced from 100 to 50, andDEVIATION_INTERVAL
has been increased from 1000 to 2000. Ensure these values align with the expected behavior and system requirements.
33-40
: New constants added.The new constants seem well-defined and self-explanatory. Ensure they are used correctly and consistently within the codebase.
130-131
: New field added toReporter
struct.The
deviationThreshold
field has been added to theReporter
struct. Ensure this field is correctly initialized and used within the codebase.node/pkg/reporter/utils.go (3)
Line range hint
21-30
:
New parameter added toGetDeviatingAggregates
.The
threshold
parameter has been added toGetDeviatingAggregates
. Ensure this parameter is correctly passed and used within the function.
37-43
: New parameter added toShouldReportDeviation
.The
threshold
parameter has been added toShouldReportDeviation
. Ensure this parameter is correctly passed and used within the function.
363-372
: New functionGetDeviationThreshold
.The
GetDeviationThreshold
function is well-implemented and calculates the deviation threshold based on thesubmissionInterval
. Ensure the logic aligns with the expected behavior.node/pkg/reporter/reporter.go (2)
50-56
: InitializedeviationThreshold
inNewReporter
.The
deviationThreshold
is correctly initialized using theGetDeviationThreshold
function and assigned to thedeviationThreshold
field in theReporter
struct.
318-318
: PassdeviationThreshold
toGetDeviatingAggregates
.The
deviationThreshold
is correctly passed as an argument to theGetDeviatingAggregates
function.
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.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
node/pkg/reporter/reporter_test.go (1)
510-526
: LGTM! Consider adding more edge cases.The new
TestGetDeviationThreshold
function comprehensively tests various scenarios for the deviation threshold calculation.Consider adding more edge cases, such as very large or very small time durations, to ensure robustness.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- node/pkg/reporter/reporter_test.go (3 hunks)
Additional comments not posted (2)
node/pkg/reporter/reporter_test.go (2)
503-508
: LGTM! Verify correctness of threshold parameter.The additions to the
TestShouldReportDeviation
function correctly test theShouldReportDeviation
function with the new deviation threshold parameter.However, ensure that the threshold values used in the assertions are appropriate and reflect real-world scenarios.
553-553
: LGTM! Verify correctness of threshold parameter.The additions to the
TestGetDeviatingAggregates
function correctly test theGetDeviatingAggregates
function with the new deviation threshold parameter.However, ensure that the threshold values used in the assertions are appropriate and reflect real-world scenarios.
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.
lgtm!
Description
set deviation threshold based on submission interval
https://go.dev/play/p/0NuWyupgKRf
Type of change
Please delete options that are not relevant.
Checklist before requesting a review
Deployment