Skip to content
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

Feature cluster matching #1862

Merged
merged 10 commits into from
Jan 9, 2025
34 changes: 34 additions & 0 deletions migration/1728554628004-AddEstimatedClusterMatching.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { MigrationInterface, QueryRunner } from 'typeorm';

export class AddEstimatedClusterMatching1728554628004
implements MigrationInterface
{
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`
CREATE TABLE estimated_cluster_matching (
id SERIAL PRIMARY KEY,
project_id INT NOT NULL,
qf_round_id INT NOT NULL,
matching DOUBLE PRECISION NOT NULL
);
`);
Comment on lines +8 to +14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add foreign key constraints and audit fields

The table definition is missing important constraints and audit fields.

 CREATE TABLE estimated_cluster_matching (
   id SERIAL PRIMARY KEY,
-  project_id INT NOT NULL,
-  qf_round_id INT NOT NULL,
+  project_id INT NOT NULL REFERENCES projects(id),
+  qf_round_id INT NOT NULL REFERENCES qf_rounds(id),
   matching DOUBLE PRECISION NOT NULL
+  created_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP,
+  updated_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP,
+  deleted_at TIMESTAMP WITH TIME ZONE
 );

Committable suggestion skipped: line range outside the PR's diff.


// Create indexes on the new table
await queryRunner.query(`
CREATE INDEX estimated_cluster_matching_project_id_qfround_id
ON estimated_cluster_matching (project_id, qf_round_id);
`);

await queryRunner.query(`
CREATE INDEX estimated_cluster_matching_matching
ON estimated_cluster_matching (matching);
`);
}

public async down(queryRunner: QueryRunner): Promise<void> {
// Revert changes if necessary by dropping the table and restoring the view
await queryRunner.query(`
DROP TABLE IF EXISTS estimated_cluster_matching;
`);
}
}
17 changes: 17 additions & 0 deletions src/adapters/adaptersFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import { DonationSaveBackupMockAdapter } from './donationSaveBackup/DonationSave
import { SuperFluidAdapter } from './superFluid/superFluidAdapter';
import { SuperFluidMockAdapter } from './superFluid/superFluidMockAdapter';
import { SuperFluidAdapterInterface } from './superFluid/superFluidAdapterInterface';
import { CocmAdapter } from './cocmAdapter/cocmAdapter';
import { CocmMockAdapter } from './cocmAdapter/cocmMockAdapter';
import { CocmAdapterInterface } from './cocmAdapter/cocmAdapterInterface';

const discordAdapter = new DiscordAdapter();
const googleAdapter = new GoogleAdapter();
Expand Down Expand Up @@ -147,3 +150,17 @@ export const getSuperFluidAdapter = (): SuperFluidAdapterInterface => {
return superFluidMockAdapter;
}
};

const clusterMatchingAdapter = new CocmAdapter();
const clusterMatchingMockAdapter = new CocmMockAdapter();

export const getClusterMatchingAdapter = (): CocmAdapterInterface => {
switch (process.env.CLUSTER_MATCHING_ADAPTER) {
case 'clusterMatching':
return clusterMatchingAdapter;
case 'mock':
return clusterMatchingMockAdapter;
default:
return clusterMatchingMockAdapter;
}
};
46 changes: 46 additions & 0 deletions src/adapters/cocmAdapter/cocmAdapter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import axios from 'axios';
import {
CocmAdapterInterface,
EstimatedMatchingInput,
ProjectsEstimatedMatchings,
} from './cocmAdapterInterface';
import { logger } from '../../utils/logger';
import { i18n, translationErrorMessagesKeys } from '../../utils/errorMessages';

export class CocmAdapter implements CocmAdapterInterface {
private ClusterMatchingURL;

constructor() {
this.ClusterMatchingURL =
process.env.CLUSTER_MATCHING_API_URL || 'localhost';
Comment on lines +14 to +15
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Security: Improve default configuration for production use

The default URL of 'localhost' could be problematic in production. Consider:

  1. Making the environment variable required
  2. Using a more secure default URL if needed
  3. Adding URL validation
-    this.ClusterMatchingURL =
-      process.env.CLUSTER_MATCHING_API_URL || 'localhost';
+    if (!process.env.CLUSTER_MATCHING_API_URL) {
+      throw new Error('CLUSTER_MATCHING_API_URL environment variable is required');
+    }
+    this.ClusterMatchingURL = process.env.CLUSTER_MATCHING_API_URL;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this.ClusterMatchingURL =
process.env.CLUSTER_MATCHING_API_URL || 'localhost';
if (!process.env.CLUSTER_MATCHING_API_URL) {
throw new Error('CLUSTER_MATCHING_API_URL environment variable is required');
}
this.ClusterMatchingURL = process.env.CLUSTER_MATCHING_API_URL;

}

async fetchEstimatedClusterMatchings(
matchingDataInput: EstimatedMatchingInput,
): Promise<ProjectsEstimatedMatchings> {
try {
const result = await axios.post(
this.ClusterMatchingURL,
matchingDataInput,
{
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
},
},
);
Comment on lines +22 to +31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add timeout and retry mechanism for API calls

The axios request lacks timeout configuration and retry mechanism, which could lead to hanging requests or unnecessary failures.

+    const axiosConfig = {
+      timeout: 5000, // 5 seconds
+      headers: {
+        Accept: 'application/json',
+        'Content-Type': 'application/json',
+      },
+      retry: 3,
+      retryDelay: (retryCount) => retryCount * 1000, // exponential backoff
+    };
     const result = await axios.post(
       this.ClusterMatchingURL,
       matchingDataInput,
-      {
-        headers: {
-          Accept: 'application/json',
-          'Content-Type': 'application/json',
-        },
-      },
+      axiosConfig,
     );

Committable suggestion skipped: line range outside the PR's diff.

if (result?.data?.error !== null) {
logger.error('clusterMatchingApi error', result.data.error);
throw new Error(
i18n.__(translationErrorMessagesKeys.CLUSTER_MATCHING_API_ERROR),
);
}
return result.data;
Comment on lines +32 to +38
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve error handling logic

The error check result?.data?.error !== null is ambiguous and might not catch all error cases.

-    if (result?.data?.error !== null) {
+    if (!result?.data || result.data.error) {
       logger.error('clusterMatchingApi error', result.data.error);
       throw new Error(
         i18n.__(translationErrorMessagesKeys.CLUSTER_MATCHING_API_ERROR),
       );
     }
+    // Validate response structure matches ProjectsEstimatedMatchings
+    if (!Array.isArray(result.data?.matching_data)) {
+      logger.error('Invalid response structure', result.data);
+      throw new Error(
+        i18n.__(translationErrorMessagesKeys.CLUSTER_MATCHING_API_ERROR),
+      );
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (result?.data?.error !== null) {
logger.error('clusterMatchingApi error', result.data.error);
throw new Error(
i18n.__(translationErrorMessagesKeys.CLUSTER_MATCHING_API_ERROR),
);
}
return result.data;
if (!result?.data || result.data.error) {
logger.error('clusterMatchingApi error', result.data.error);
throw new Error(
i18n.__(translationErrorMessagesKeys.CLUSTER_MATCHING_API_ERROR),
);
}
// Validate response structure matches ProjectsEstimatedMatchings
if (!Array.isArray(result.data?.matching_data)) {
logger.error('Invalid response structure', result.data);
throw new Error(
i18n.__(translationErrorMessagesKeys.CLUSTER_MATCHING_API_ERROR),
);
}
return result.data;

} catch (e) {
logger.error('clusterMatchingApi error', e);
throw new Error(
i18n.__(translationErrorMessagesKeys.CLUSTER_MATCHING_API_ERROR),
);
}
}
}
49 changes: 49 additions & 0 deletions src/adapters/cocmAdapter/cocmAdapterInterface.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Example Data
// {
// "matching_data": [
// {
// "matching_amount": 83.25,
// "matching_percent": 50.0,
// "project_name": "Test1",
// "strategy": "COCM"
// },
// {
// "matching_amount": 83.25,
// "matching_percent": 50.0,
// "project_name": "Test3",
// "strategy": "COCM"
// }
// ]
// }

export interface ProjectsEstimatedMatchings {
matching_data: {
matching_amount: number;
matching_percent: number;
project_name: string;
strategy: string;
}[];
}

export interface EstimatedMatchingInput {
votes_data: [
{
voter: string;
payoutAddress: string;
amountUSD: number;
project_name: string;
score: number;
},
];
strategy: string;
min_donation_threshold_amount: number;
matching_cap_amount: number;
matching_amount: number;
passport_threshold: number;
}

export interface CocmAdapterInterface {
fetchEstimatedClusterMatchings(
matchingDataInput: EstimatedMatchingInput,
): Promise<ProjectsEstimatedMatchings>;
}
27 changes: 27 additions & 0 deletions src/adapters/cocmAdapter/cocmMockAdapter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import {
CocmAdapterInterface,
ProjectsEstimatedMatchings,
} from './cocmAdapterInterface';

export class CocmMockAdapter implements CocmAdapterInterface {
async fetchEstimatedClusterMatchings(
_matchingDataInput,
): Promise<ProjectsEstimatedMatchings> {
return {
matching_data: [
{
matching_amount: 83.25,
matching_percent: 50.0,
project_name: 'Test1',
strategy: 'COCM',
},
{
matching_amount: 83.25,
matching_percent: 50.0,
project_name: 'Test3',
strategy: 'COCM',
},
],
};
}
}
2 changes: 2 additions & 0 deletions src/entities/entities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import { ProjectSocialMedia } from './projectSocialMedia';
import { DraftRecurringDonation } from './draftRecurringDonation';
import { UserQfRoundModelScore } from './userQfRoundModelScore';
import { ProjectGivbackRankView } from './ProjectGivbackRankView';
import { EstimatedClusterMatching } from './estimatedClusterMatching';

export const getEntities = (): DataSourceOptions['entities'] => {
return [
Expand Down Expand Up @@ -86,6 +87,7 @@ export const getEntities = (): DataSourceOptions['entities'] => {
PowerSnapshot,
PowerBalanceSnapshot,
PowerBoostingSnapshot,
EstimatedClusterMatching,

// View
UserProjectPowerView,
Expand Down
41 changes: 41 additions & 0 deletions src/entities/estimatedClusterMatching.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { Field, ObjectType } from 'type-graphql';
import {
Column,
Index,
PrimaryGeneratedColumn,
BaseEntity,
Entity,
ManyToOne,
JoinColumn,
} from 'typeorm';
import { Project } from './project';

@Entity('estimated_cluster_matching')
@Index('estimated_cluster_matching_project_id_qfround_id', [
'projectId',
'qfRoundId',
])
@Index('estimated_cluster_matching_matching', ['matching'])
@ObjectType()
export class EstimatedClusterMatching extends BaseEntity {
@Field()
@PrimaryGeneratedColumn()
id: number; // New primary key

@Field(_type => Project)
@ManyToOne(_type => Project, project => project.projectEstimatedMatchingView)
@JoinColumn({ referencedColumnName: 'id' })
project: Project;

@Field()
@Column()
projectId: number;

@Field()
@Column()
qfRoundId: number;

@Field()
@Column('double precision')
matching: number;
Comment on lines +38 to +40
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider Decimal Precision for Monetary Fields

double precision might introduce floating-point inaccuracies when dealing with money-like values. Consider a decimal type or a scaled integer if matching is meant for financial calculations.

}
31 changes: 25 additions & 6 deletions src/entities/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,16 @@ import { FeaturedUpdate } from './featuredUpdate';
import { getHtmlTextSummary } from '../utils/utils';
import { QfRound } from './qfRound';
import {
getQfRoundTotalSqrtRootSumSquared,
getProjectDonationsSqrtRootSum,
findActiveQfRound,
getProjectDonationsSqrtRootSum,
getQfRoundTotalSqrtRootSumSquared,
} from '../repositories/qfRoundRepository';
import { EstimatedMatching } from '../types/qfTypes';
import { Campaign } from './campaign';
import { ProjectEstimatedMatchingView } from './ProjectEstimatedMatchingView';
import { AnchorContractAddress } from './anchorContractAddress';
import { ProjectSocialMedia } from './projectSocialMedia';
import { EstimatedClusterMatching } from './estimatedClusterMatching';

// eslint-disable-next-line @typescript-eslint/no-var-requires
const moment = require('moment');
Expand Down Expand Up @@ -501,9 +502,10 @@ export class Project extends BaseEntity {
async estimatedMatching(): Promise<EstimatedMatching | null> {
const activeQfRound = await findActiveQfRound();
if (!activeQfRound) {
// TODO should move it to materialized view
return null;
}
const matchingPool = activeQfRound.allocatedFund;

const projectDonationsSqrtRootSum = await getProjectDonationsSqrtRootSum(
this.id,
activeQfRound.id,
Expand All @@ -513,12 +515,29 @@ export class Project extends BaseEntity {
activeQfRound.id,
);

const matchingPool = activeQfRound.allocatedFund;
const estimatedClusterMatching =
await EstimatedClusterMatching.createQueryBuilder('matching')
.where('matching."projectId" = :projectId', { projectId: this.id })
.andWhere('matching."qfRoundId" = :qfRoundId', {
qfRoundId: activeQfRound.id,
})
.getOne();

let matching: number;
if (!estimatedClusterMatching) matching = 0;

if (!estimatedClusterMatching) {
matching = 0;
} else {
matching = estimatedClusterMatching.matching;
}

// Facilitate migration in frontend return empty values for now
return {
projectDonationsSqrtRootSum,
allProjectsSum,
projectDonationsSqrtRootSum: projectDonationsSqrtRootSum,
allProjectsSum: allProjectsSum,
matchingPool,
matching,
};
}

Expand Down
22 changes: 22 additions & 0 deletions src/repositories/donationRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,28 @@ import { ORGANIZATION_LABELS } from '../entities/organization';
import { AppDataSource } from '../orm';
import { getPowerRound } from './powerRoundRepository';

export const exportClusterMatchingDonationsFormat = async (
qfRoundId: number,
) => {
return await Donation.query(
`
SELECT
d."fromWalletAddress" AS voter,
d."toWalletAddress" AS "payoutAddress",
d."valueUsd" AS "amountUSD",
p."title" AS "project_name",
d."qfRoundUserScore" AS score
FROM
donation d
INNER JOIN
project p ON d."projectId" = p."id"
WHERE
d."qfRoundId" = $1
`,
[qfRoundId],
);
};

export const fillQfRoundDonationsUserScores = async (): Promise<void> => {
await Donation.query(`
UPDATE donation
Expand Down
4 changes: 3 additions & 1 deletion src/resolvers/projectResolver.allProject.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,13 @@ import { InstantPowerBalance } from '../entities/instantPowerBalance';
import { saveOrUpdateInstantPowerBalances } from '../repositories/instantBoostingRepository';
import { updateInstantBoosting } from '../services/instantBoostingServices';
import { QfRound } from '../entities/qfRound';
import { calculateEstimatedMatchingWithParams } from '../utils/qfUtils';
// import { calculateEstimatedMatchingWithParams } from '../utils/qfUtils';
import { refreshProjectEstimatedMatchingView } from '../services/projectViewsService';
import { addOrUpdatePowerSnapshotBalances } from '../repositories/powerBalanceSnapshotRepository';
import { findPowerSnapshots } from '../repositories/powerSnapshotRepository';
import { ChainType } from '../types/network';
import { ORGANIZATION_LABELS } from '../entities/organization';
import { calculateEstimatedMatchingWithParams } from '../utils/qfUtils';

// search and filters
describe('all projects test cases --->', allProjectsTestCases);
Expand Down Expand Up @@ -2215,6 +2216,7 @@ function allProjectsTestCases() {
p => Number(p.id) === project2.id,
);

// New estimated matching wont calculate it here
const project1EstimatedMatching =
await calculateEstimatedMatchingWithParams({
matchingPool: firstProject.estimatedMatching.matchingPool,
Expand Down
Loading
Loading