-
Notifications
You must be signed in to change notification settings - Fork 1
Added lifecycle hooks to solver table #85
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
Changes from 4 commits
5892d45
a7e2fb4
7ea0948
b15c260
f36bff8
c41513f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,181 @@ | ||
| import { errors } from "@strapi/utils"; | ||
| import { getMonthsDifference } from "../../../../utils/date"; | ||
|
|
||
| const COW_BONDING_POOL_SERVICE_FEE_TH: number = 6; | ||
| const COLOCATED_BONDING_POOL_SERVICE_FEE_TH: number = 3; | ||
|
|
||
| export default { | ||
| async beforeCreate(event) { | ||
| try { | ||
| await updateActiveNetworks(event); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be done after updating or creating? |
||
| await updateServiceFeeEnabled(event); | ||
| } catch (error) { | ||
| console.error("Error in beforeCreate:", error); | ||
| throw new errors.ValidationError(error.message || "Error processing solver data"); | ||
| } | ||
| }, | ||
|
|
||
| async beforeUpdate(event) { | ||
| try { | ||
| await updateActiveNetworks(event); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since these updates and creates will be done through Dagster, what's the advantage of processing this through the lifecycle vs adding this update as a step in the same Dagster job that will do the update that triggers this hook? |
||
| await updateServiceFeeEnabled(event); | ||
| } catch (error) { | ||
| console.error("Error in beforeUpdate:", error); | ||
| throw new errors.ValidationError(error.message || "Error processing solver data"); | ||
| } | ||
| }, | ||
| }; | ||
|
|
||
| interface SolverData { | ||
| activeNetworks?: string[]; | ||
| hasActiveNetworks?: boolean; | ||
| isServiceFeeEnabled?: boolean; | ||
| solver_networks?: any; | ||
| } | ||
|
|
||
| interface StrapiEvent { | ||
| params: { | ||
| data: SolverData; | ||
| where?: { | ||
| id: number; | ||
| }; | ||
| }; | ||
| result?: { | ||
| id: number; | ||
| }; | ||
| } | ||
|
|
||
| async function updateActiveNetworks(event: StrapiEvent) { | ||
| const { data, where } = event.params; | ||
| const solverData: SolverData = data; | ||
|
|
||
| if (where && !data.solver_networks) { | ||
| try { | ||
| const solver = await strapi.entityService.findOne( | ||
| 'api::solver.solver', | ||
| where.id, | ||
| { populate: ['solver_networks.network'] } | ||
| ); | ||
|
|
||
| if (solver) { | ||
| await calculateActiveNetworks(solver as Solver, solverData); | ||
| } | ||
| } catch (error) { | ||
| console.error(`Error fetching solver data for id ${where.id}:`, error); | ||
| throw new errors.ApplicationError(`Failed to fetch solver data: ${error.message}`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| interface Solver { | ||
| id: number; | ||
| solver_networks?: Array<{ | ||
| active?: boolean; | ||
| network?: { | ||
| name?: string; | ||
| }; | ||
| }>; | ||
| solver_bonding_pools?: Array<{ | ||
| name?: string; | ||
| joinedOn?: string; | ||
| }>; | ||
| isColocated?: string; | ||
| } | ||
|
|
||
| async function calculateActiveNetworks(solver: Solver, data: SolverData): Promise<void> { | ||
| if (!solver.solver_networks) { | ||
| data.activeNetworks = []; | ||
| data.hasActiveNetworks = false; | ||
| return; | ||
| } | ||
|
|
||
| // Filter active networks and extract their names | ||
| const activeNetworkNames = solver.solver_networks | ||
| .filter(network => network.active) | ||
| .map(network => network.network?.name) | ||
| .filter(Boolean) as string[]; // Remove any undefined values | ||
|
|
||
| data.activeNetworks = activeNetworkNames; | ||
| data.hasActiveNetworks = activeNetworkNames.length > 0; | ||
| } | ||
|
|
||
| async function updateServiceFeeEnabled(event: StrapiEvent): Promise<void> { | ||
| const { data, where } = event.params; | ||
| const solverData: SolverData = data; | ||
|
|
||
| if (where) { | ||
| try { | ||
| const solver = await strapi.entityService.findOne( | ||
| 'api::solver.solver', | ||
| where.id, | ||
| { | ||
| populate: { | ||
| solver_bonding_pools: { | ||
| fields: ['name', 'joinedOn'] | ||
| } | ||
| } | ||
| } | ||
| ); | ||
|
|
||
| if (solver) { | ||
| await setServiceFeeEnabled(solver as Solver, solverData); | ||
| } | ||
| } catch (error) { | ||
| console.error(`Error fetching solver data for service fee calculation (id ${where.id}):`, error); | ||
| throw new errors.ApplicationError(`Failed to fetch solver data for service fee calculation: ${error.message}`); | ||
| } | ||
| } | ||
| // For create operation, we handle it in afterCreate since we need the ID | ||
| } | ||
|
|
||
| /** | ||
| * Determines if a solver is eligible for service fees based on bonding pool criteria. | ||
| * Pure function that checks eligibility based on time thresholds: | ||
| * - 6+ months for non-colocated CoW pools | ||
| * - 3+ months for colocated pools | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious, is this comming from EIP, if so. Could you add it? |
||
| * @param solver The solver entity to check | ||
| * @returns Boolean indicating if the solver is eligible for service fees | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TBH, i don't know what is service fee exactly. I assume this is not protocol fee related. To avoid these things, if you give more context in the PR you avoid people that is not familiar with your current work to get a proper idea |
||
| */ | ||
| function isEligibleForServiceFee(solver: Solver): boolean { | ||
| if (!solver.solver_bonding_pools || solver.solver_bonding_pools.length === 0) { | ||
| return false; | ||
| } | ||
|
|
||
| const currentDate = new Date(); | ||
|
|
||
| for (const bondingPool of solver.solver_bonding_pools) { | ||
| if (!bondingPool.joinedOn) { | ||
| continue; | ||
| } | ||
|
|
||
| const joinedDate = new Date(bondingPool.joinedOn); | ||
| const monthsDifference = getMonthsDifference(joinedDate, currentDate); | ||
|
|
||
| if (bondingPool.name === "CoW" && solver.isColocated === "No") { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should do this in a lifecycle. Having this logic in the lifecycle means that the BL for updating the CMS is split into two different places, both in Dagster and the CMS itself. That adds unnecessary complexity and makes it harder to maintain in the future. I think it would be better to do this in Dagster and keep a separation where data is transformed and updated through Dagster and these lifecycles handle simple aggregation. |
||
| if (monthsDifference >= COW_BONDING_POOL_SERVICE_FEE_TH) { | ||
| return true; | ||
| } | ||
| } | ||
| else if (solver.isColocated === "Yes") { | ||
| if (monthsDifference >= COLOCATED_BONDING_POOL_SERVICE_FEE_TH) { | ||
| return true; | ||
| } | ||
| } | ||
| // For partial colocated, we'll treat it as not colocated | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the isServiceFeeEnabled flag in the solver data based on eligibility. | ||
| * Uses the pure function isEligibleForServiceFee to determine eligibility. | ||
| */ | ||
| async function setServiceFeeEnabled(solver: Solver, data: SolverData): Promise<void> { | ||
| try { | ||
| data.isServiceFeeEnabled = isEligibleForServiceFee(solver); | ||
| } catch (error) { | ||
| console.error(`Error setting service fee enabled status:`, error); | ||
| throw new errors.ApplicationError(`Failed to set service fee status: ${error.message}`); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| /** | ||
| * Date utility functions | ||
| */ | ||
|
|
||
| /** | ||
| * Calculate the number of months between two dates | ||
| * @param startDate The start date | ||
| * @param endDate The end date | ||
| * @returns The number of months between the two dates | ||
| */ | ||
| export function getMonthsDifference(startDate: Date, endDate: Date): number { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if this is the implementation you want. Si hard to review If you have: Your algo will show 1 month. But it was just 1 day. Is this correct? If you have: It will return 0 months, but its just 1 day too.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right this is not the intended behavior, it should show true months difference. |
||
| const years = endDate.getFullYear() - startDate.getFullYear(); | ||
| const months = endDate.getMonth() - startDate.getMonth(); | ||
| return years * 12 + months; | ||
| } | ||
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.
Those lifecycle hooks are applied to
solvercollection; shouldn't they be applied tosolver-networkcollection?For example, you have a
solveritem andsolver-networkwhich are coupled. Once you delete thesolver-networkitem, it should update thesolveritem fields.