-
Notifications
You must be signed in to change notification settings - Fork 10
Refactor CLPool event handling to aggregate-driven architecture with business logic isolation #364
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
Conversation
…siness logic isolation
- event: CollectFees(address indexed recipient, uint128 amount0, uint128 amount1) | ||
- event: Flash(address indexed sender, address indexed recipient, uint256 amount0, uint256 amount1, uint256 paid0, uint256 paid1) | ||
- event: IncreaseObservationCardinalityNext(uint16 observationCardinalityNextOld, uint16 observationCardinalityNextNew) | ||
- event: Initialize(uint160 sqrtPriceX96, int24 tick) |
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.
this event didn't have any useful information afaik
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.
deleted a bunch of CLPool entities and added some fields on User and pool entities
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.
changed the approach here
update functions of the user and pool entitities compute the final values: current + diff for cumulative quantities
For other quantities, just substitute current with diff
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.
hmmm I don't understand, was it previously assigning diff to cumulative values?
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.
Yes exactly. The previous code simply substituted all values (cumulative and not cumulative) of current
by the values of diff
.
This is right for non-cumulative variables but it was wrong for cumulative
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.
okay so this is a bug fix rather than a refactor right?
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.
centralised user stats updates into a single 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.
standardized the logic flow accross all event handlers + deleted event-driven entities that aren't used now
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.
business logic for CLPoolBurn
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.
cleaning house
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.
business logic for CLPoolFlash
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.
refactored this taking into account that event-driven entities no longer exist
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.
same as above comment
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.
standardize "diff" definition -> diff should always be only the changes derived from the event
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.
standardize "diff" definition -> diff should always be only the changes derived from the event
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.
standardize "diff" definition -> diff should always be only the changes derived from the event
Implements:
Removed individual CLPool event entities (
CLPool_Burn
,CLPool_Collect
,CLPool_CollectFees
,CLPool_Flash
,CLPool_Mint
,CLPool_Swap
) and consolidated data into LiquidityPoolAggregator and UserStatsPerPool entitiesBusiness logic isolation: Extracted complex calculations into dedicated logic files (
CLPoolBurnLogic
,CLPoolCollectLogic
,CLPoolFlashLogic
, etc.) with consistent processCLPool* interfaces for improved testability and maintainabilityReplaced specific update functions with generic
updateUserStatsPerPool
and enhancedupdateLiquidityPoolAggregator
to properly handle cumulative vs non-cumulative fieldsEnhanced schema for CL pools: Added CL-specific fields (feeProtocol0/1, observationCardinalityNext, totalFlashLoanFees0/1/USD, totalFlashLoanVolumeUSD, numberOfFlashLoans) to pool aggregators and user stats
Refactored all tests with exact assertions, proper mocking, and consistent patterns to ensure reliability of the new architecture
Standardized all CLPool handlers to follow the same data loading → processing → updating flow for improved maintainability