-
Notifications
You must be signed in to change notification settings - Fork 117
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
[IND-500]: Create Aggregate trading rewards roundtable task outline #917
Conversation
WalkthroughThe recent update introduces a feature for aggregating trading rewards over a given time period. This involves setting up tests for the feature and implementing the core functionality that calculates rewards based on trading data, and then saves the aggregated information. Additionally, new configuration parameters and a conditional loop have been added to control the aggregation process. Changes
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? TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- indexer/services/roundtable/tests/tasks/aggregate-trading-rewards.test.ts (1 hunks)
- indexer/services/roundtable/src/tasks/aggregate-trading-rewards.ts (1 hunks)
Additional comments: 3
indexer/services/roundtable/__tests__/tasks/aggregate-trading-rewards.test.ts (1)
- 1-4: The import statements are correctly structured and include necessary modules for database operations and test mocks.
indexer/services/roundtable/src/tasks/aggregate-trading-rewards.ts (2)
1-5: Import statements are correctly structured and include necessary types from the database and the
luxon
library for date-time manipulation.14-21: The
Interval
andSortedTradingRewardData
interfaces are well-defined and provide a clear structure for the data used in the task.
describe('aggregate-trading-rewards', () => { | ||
beforeAll(async () => { | ||
await dbHelpers.migrate(); | ||
await dbHelpers.clearData(); | ||
}); | ||
|
||
beforeEach(async () => { | ||
await testMocks.seedData(); | ||
}); | ||
|
||
afterEach(async () => { | ||
await dbHelpers.clearData(); | ||
jest.resetAllMocks(); | ||
}); | ||
|
||
afterAll(async () => { | ||
await dbHelpers.teardown(); | ||
}); |
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.
Lifecycle hooks for setting up and tearing down the test environment are well-defined. However, there are no actual test cases defined within the describe
block. Ensure that test cases are added to validate the functionality of the aggregate-trading-rewards
task.
export default async function runTask(): Promise<void> { | ||
// TODO(IND-499): Add resetting aggregation data when cache is empty | ||
const interval: Interval | undefined = await getTradingRewardDataToProcessInterval(); | ||
|
||
const tradingRewardData: TradingRewardFromDatabase[] = await getTradingRewardDataToProcess(interval); | ||
const sortedTradingRewardData: SortedTradingRewardData = sortTradingRewardData(tradingRewardData); | ||
await updateTradingRewardsAggregation(sortedTradingRewardData); | ||
// TODO(IND-499): Update AggregateTradingRewardsProcessedCache | ||
} | ||
|
||
async function getTradingRewardDataToProcessInterval(): Promise<Interval> { | ||
const latestBlock: BlockFromDatabase = await BlockTable.getLatest(); | ||
|
||
// TODO(IND-499): Setup AggregateTradingRewardsProcessedCache for start time and add end time | ||
return { | ||
start: DateTime.fromISO(latestBlock.time), | ||
end: DateTime.fromISO(latestBlock.time), | ||
}; | ||
} | ||
|
||
async function getTradingRewardDataToProcess(interval: Interval): Promise<TradingRewardFromDatabase[]> { | ||
// TODO: Implement | ||
return []; | ||
} | ||
|
||
function sortTradingRewardData(tradingRewardData: TradingRewardFromDatabase[]): SortedTradingRewardData { | ||
// TODO: Implement | ||
return {}; | ||
} | ||
|
||
async function updateTradingRewardsAggregation(sortedTradingRewardData: SortedTradingRewardData): Promise<void> { | ||
// TODO: Implement | ||
} |
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 runTask
function and related helper functions contain several TODO
comments indicating that the implementation is not yet complete. It is crucial to implement these functions and remove the TODO
comments before merging the PR to ensure the task is functional.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- indexer/services/roundtable/tests/tasks/aggregate-trading-rewards.test.ts (1 hunks)
- indexer/services/roundtable/src/tasks/aggregate-trading-rewards.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- indexer/services/roundtable/tests/tasks/aggregate-trading-rewards.test.ts
- indexer/services/roundtable/src/tasks/aggregate-trading-rewards.ts
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- indexer/services/roundtable/tests/tasks/aggregate-trading-rewards.test.ts (1 hunks)
- indexer/services/roundtable/src/config.ts (2 hunks)
- indexer/services/roundtable/src/index.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- indexer/services/roundtable/tests/tasks/aggregate-trading-rewards.test.ts
Additional comments: 4
indexer/services/roundtable/src/index.ts (2)
11-11: The import of
aggregateTradingRewardsTasks
is consistent with the existing module import pattern.125-131: Ensure that the configuration parameter
LOOPS_ENABLED_AGGREGATE_TRADING_REWARDS
is set correctly in all environments to avoid unintended behavior.indexer/services/roundtable/src/config.ts (2)
45-45: The addition of the
LOOPS_ENABLED_AGGREGATE_TRADING_REWARDS
configuration parameter with a default value oftrue
is consistent with the pattern of feature flags in the config.78-80: The addition of the
LOOPS_INTERVAL_MS_AGGREGATE_TRADING_REWARDS
configuration parameter with a default value ofONE_MINUTE_IN_MILLISECONDS
is consistent with the pattern of loop interval settings in the config.
Changelist
Create Aggregate trading rewards roundtable task outline
Test Plan
none
Author/Reviewer Checklist
state-breaking
label.indexer-postgres-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.